-
Notifications
You must be signed in to change notification settings - Fork 1.4k
PHPORM-274 List search indexes in Schema::getIndexes() introspection method
#3233
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I plan to test all the search index features in this class. It is also in #3232
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some questions but LGTM. Feel free to add a test for dynamic field mappings if you wish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per Atlas Vector Search Index Fields, I expect this might return field paths instead of field names. Is that going to cause problems with whatever consumes this?
I assume it may be consistent with how the basic listIndexes() query above runs (using array_keys($index->getKey()) but I'm not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As soon as it is a string, it can be displayed.
src/Schema/Builder.php
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need better reporting for dynamic indexes, or is there no good way to express that given the expected format of $indexList elements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All I can do is returning "dynamic" as a column name. This is purely informative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning dynamic as a column name seems undesirable, as then it might be indistinguishable from a real index with a dynamic field mapped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alternative is replace the type with search, dynamic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is outside the diff but I think it's worth mentioning since I see you use an === to check for ['_id' => 1]. If there is ever an issue with that, you may need to relax the comparison on the numeric value. IIRC, mongos used to sometimes return command results as 1.0 instead of 1 and I think there may be similar inconsistencies for index definitions.
That said, it's probably fine to just wait and see if this is ever reported before going out of your way to address it now. It's quite possible that it's a non-issue on recent server versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The triple === is enforced by phpcs. We will fix it if a new server version produces an incompatible value, but I doubt it.
Example of output:
❯ php artisan db:table comments
comments ................................................................................
Columns ............................................................................... 7
Size ........................................................................... 52.00 KB
Column ............................................................................. Type
id objectId, objectId .......................................................... objectId
author_id string, nullable ....................................................... string
content string, nullable ......................................................... string
created_at date, nullable .......................................................... date
post_id string, nullable ......................................................... string
published_at date, nullable ........................................................ date
updated_at date, nullable .......................................................... date
Index ...................................................................................
_id_ _id ........................................................................ primary
vector_index my_vector ..................................................... vectorSearch
default dynamic .................................................................. search
search dynamic, body ............................................................. search
3cac0ec to
45603fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
latestDefinition.mappings.dynamicis always defined.dynamicword is added to the list for columns names it there is also explicit mapping.
Fix PHPORM-274
Search indexes are a distinct type of index that are exposed using a specific method. Listing the search indexes in
Schema::getIndexes()so they are listed by the commandphp artisan db:table.