-
Notifications
You must be signed in to change notification settings - Fork 48
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
Feat query gen 2.2 #162
Feat query gen 2.2 #162
Conversation
Feat: Improved performance
bce0d1a
to
97754fb
Compare
/** | ||
* Helper method to create Query with contains method | ||
*/ | ||
public static function contains(string $attribute, $value): self |
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.
Should contains accept array $values
or 1 $value
? Probably an array, right? Since you may want to check if the array attribute contains value1 or value2?
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.
An array sounds good to me
/** | ||
* Helper method to create Query with search method | ||
*/ | ||
public static function search(string $attribute, $value): self |
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.
Right now, we can do an OR search by passing 1 string with each value separated by space. Should this signature be changed to array $values
, though, and then we do implode(' ', $values)
server side?
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.
On MYSQL Fulltext can take a few attributes such as
SELECT * FROM collection WHERE MATCH(column1, column2) AGAINST ('text')
I know today there is no support, only a single attribute
/** | ||
* Helper method to create Query with cursorBefore method | ||
*/ | ||
public static function cursorBefore(string $value): self |
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.
Looks like this string $value
should be Document $value
, right?
/** | ||
* Helper method to create Query with orderDesc method | ||
*/ | ||
public static function orderDesc(string $attribute): self |
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.
In some of our test cases, we order by "natural order" where we specify direction but no attribute. Is it to weird to do Query::orderDesc('')
for those cases?
Based on feedback in previous PR, update Query class to be used like:
Update parsing of queries to be like: