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

Index compilation broken with Laravel 10 #53

Closed
hendrikheil opened this issue Mar 24, 2023 · 3 comments
Closed

Index compilation broken with Laravel 10 #53

hendrikheil opened this issue Mar 24, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@hendrikheil
Copy link

hendrikheil commented Mar 24, 2023

We just upgraded to Laravel 10, but encountered an issue with our index compilation.
We're actually using CockroachDB, not PostgreSQL but the use-case should be identical and behave the same for both.
In our migrations we have some columns as jsonb and we're adding a GIN index on an attribute of that JSON column.

The migration looks like this:

Schema::create('examples', function (Blueprint $table) {
    $table->jsonb('meta');
    $table->rawIndex('("meta"->\'attribute\')', 'examples_meta_attribute_index')->algorithm('gin');
});

However starting with Laravel 10, expressions can't just be cast to string. This means that this line is now not applicable for all possible cases:

$columns = array_map(function (string $column): string {

I believe fixing it would be as simple as changing from string to string|Expression and adding the same functionality that already exists in laravel-query-expressions as stringize
https://github.com/tpetry/laravel-query-expressions/blob/4ca06ed9abf0ac5e52f83a613124389d4c03392b/src/Concerns/StringizeExpression.php#L12-L18

There are possibly more cases where there is currently an expectation for string but we might receive an Expression instead.

I'm happy to send a pull-request, but I'm not quite sure how one gets those fancy numbers for columns and tables that are used in other tests :)

@tpetry tpetry added the bug Something isn't working label Mar 24, 2023
@tpetry
Copy link
Owner

tpetry commented Mar 24, 2023

Will be fixed in about an hour ;)

@hendrikheil
Copy link
Author

hendrikheil commented Mar 24, 2023

Is there documentation for writing tests for this library somewhere? I'd love to be able to contribute with the projects best practices!

@tpetry tpetry closed this as completed in e13eb26 Mar 24, 2023
@tpetry
Copy link
Owner

tpetry commented Mar 24, 2023

Its fixed with release 0.27.1. The problem was not changed in Laravel 10. I never handled rawIndex definitions correctly. They are not mentioned in the documentation so I completely missed them.

There's no real documentation for tests, its just PHPUnit with some additions. The numbers at the end of everything are just random nums to prevent name clashes.

As I rewrite every PR the best approach would be just asking whether I am ok with a certain feature, work on a draft and I polish it up to fit my style. Or the way I would do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants