Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with other Postgres libraries #42

Closed
kdevan opened this issue Oct 28, 2022 · 8 comments
Closed

Compatibility with other Postgres libraries #42

kdevan opened this issue Oct 28, 2022 · 8 comments
Labels
wontfix This will not be worked on

Comments

@kdevan
Copy link

kdevan commented Oct 28, 2022

Hello! I use Postgres heavily and am very excited to see that you started this project.

Right now I have umbrellio/laravel-pg-extensions implemented and have used its "Custom Extensions" method for adding other new functionality I needed. On top of that library I have umbrellio/laravel-ltree, belamov/postgres-range, staudenmeir/laravel-adjacency-list, and with it staudenmeir/laravel-cte.

Most of the custom functionality I've added is already handled by your library, so I would just be able to remove that on my end. LTree I can just use the Eloquent helpers and your library can do the schema stuff. You also support ranges so that's easy. I do use Laravel Adjacency List heavily so I'm wondering how it would work with both libraries implementing CTE (maybe no issue if there's no naming conflict?), and then compatibility with other Postgres libraries in general.

I think the main issue I run into when I'm trying to get these libraries to work together is when there's conflicts with the PostgresConnection. And then some way for multiple libraries to extend Blueprint and Schema Grammar which is what the Custom Extension method allows for.

So far I've used the umbrellio/laravel-pg-extensions implementation of PostgresConnection and extended that. Then when there's conflicts, which I ran into with belamov/postgres-range and staudenmeir/laravel-cte, I just implement them manually using the Custom Extension method so that they do not load their versions of the PostgresConnection, and prevent them from being automatically discovered by Laravel using composer.json:

"extra": {
    "laravel": {
        "dont-discover": [
          "belamov/postgres-range",
          "staudenmeir/laravel-cte"
        ]
    }
}

Switching to your library I won't have that Custom Extension method anymore, which means I don't have a clear way to deal with conflicts, so I know that will be the main challenge in transitioning here.

Of course this is out of scope of this repository but I thought I'd ask just in case you have some insight here. Either way, I do plan to try this transition in just the next few days, so if you're not sure I'll go ahead and just update this issue with anything that I find, maybe help others in the future.

I am so excited about this library, thank you so much for starting this!

@tpetry
Copy link
Owner

tpetry commented Oct 29, 2022

Yeah that the database part is not extensible by multiple libraries is a problem in Laravel, but I didn't like the solution of umbrellio/laravel-pg-extensions because for any conflict you need to know exactly what to change. The CTE implementation is a good example as a lot of different other code parts need to be changed to support it.

So it is basically that you can only use one library to extend the database stuff. I designed my CTE implementation to be kind of similar to staudenmeir/laravel-cte. The basic syntax is the same, but the extended syntax for e.g. recursive queries or materialization is different.

So I don't know the problems you will face, but forking the other packages to change syntax a bit should work. You can share your findings about incompatibilities but I don't promise to work on them. You are entering a part of Laravel that is very fickle and it was kind of never expected that multiple plugins extend the db functionality.

@kdevan
Copy link
Author

kdevan commented Oct 29, 2022

Ah well thank you, this is very helpful context going into this.

You can share your findings about incompatibilities but I don't promise to work on them.

Of course, I'll be happy to share information here just on the chance it helps anyone else :)

@mfn
Copy link

mfn commented Oct 30, 2022

I'm also a heavy pgsql user and always wanted to use any of these, or this, library, but in our application for internal reasons we've to already use our own connection, making things even complicated.

I wonder "what would be necessary" to allow arbitrary interop of multiple libraries. Whether it's enough to provide an "PostgresConnection abstraction", a stand-alone dependency, where other libraries (this, and the other) build on top etc.

But that would require a good decision and bringing all on board, probably not easy either 😅

@tpetry
Copy link
Owner

tpetry commented Oct 30, 2022

I had the plan to make this extensibility when I started the library. But the more features I added, I understood that this idea is impossible. For adding support X maybe also feature Y needs to be changed. Its just not possible to let multiple packages extend the db driver without having so many conflicts or package Y needing to change its implementation because package X wants to add something.

@tpetry tpetry added the wontfix This will not be worked on label Dec 16, 2022
@kdevan
Copy link
Author

kdevan commented Jun 3, 2023

Just in case someone else is trying to integrate with the staudenmeir/laravel-adjacency-list library, it was much easier than I imagined. I also got umbrellio/laravel-ltree and belamov/postgres-range working which were even easier.

  1. Add the Staudenmeir\LaravelAdjacencyList\Eloquent\Traits\BuildsAdjacencyListQueries trait to a custom query builder for your model.
  2. Add this function to the query builder and modify as needed. I did the bare minimum for my own needs:
/**
 * Add a recursive common table expression to the query.
 *
 * @param  string  $name
 * @param  \Closure|\Illuminate\Database\Query\Builder|string  $query
 * @param  array|null  $columns
 */
public function withRecursiveExpression($name, $query, $columns = null, array $cycle = null): self
{
    return $this->withExpression($name, $query, [
        'recursive' => true,
    ]);
}
  1. Add the Staudenmeir\LaravelAdjacencyList\Eloquent\Traits\HasAdjacencyList to your model.
  2. Add the following to your composer.json:
"extra": {
    "laravel": {
        "dont-discover": [
          "staudenmeir/laravel-cte",
          "belamov/postgres-range"
        ]
    }
}

Usually the Staudenmeir\LaravelCte\Eloquent\QueriesExpressions; trait would get added to the model as well, so we are omitting that and including our own function with the same name, and inside that function calling the CTE function from this library.

I'm only using the recursive features of the Adjacency List library but I'm sure supporting the graph features would not be a stretch. Most likely just adding some more functions to your custom query builder.

As for umbrellio/laravel-ltree and belamov/postgres-range I just pulled in the classes I needed and let this library handle the Blueprint column types.

I am so happy to be using this library! You're amazing @tpetry.

@kdevan kdevan closed this as completed Jun 3, 2023
@tpetry
Copy link
Owner

tpetry commented Jun 3, 2023

Adding casts like belamov/postgres-range to fully replace it should be pretty easy.

But umbrellio/laravel-ltree provides a lot of stuff. And I don't know whether I want to support all of that. What are you using from this package?

@tpetry tpetry pinned this issue Jun 3, 2023
@tpetry
Copy link
Owner

tpetry commented Jun 3, 2023

I've pinned the issue as this problem may occur in the future again for other developers.

@kdevan
Copy link
Author

kdevan commented Jun 4, 2023

Adding casts like belamov/postgres-range to fully replace it should be pretty easy.

That would be great. The casts definitely make working with this data type much nicer.

But umbrellio/laravel-ltree provides a lot of stuff. And I don't know whether I want to support all of that. What are you using from this package?

Yeah, this library is pretty old and not very well maintained. I've even forked a lot of the files to modify them to work with my project. It's on my mind to do a rewrite of this package from scratch. Once I get there I'll check in with you in case you decide you'd like these features in this library. If so I'll just write it out here as a PR.

The features I use are to make creating, updating, and deleting labels eloquent friendly as well as making sure those operations happen consistently without breaking parent or child labels. I also use the feature to take a flat collection of nodes and turn it into a tree.

Basically taking these special operators and functions and making them eloquent friendly with a little magic to make common operations simpler to execute. Otherwise you need to use lots of Query Builder raw functions to make ltree work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants