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

Replace IndexOrdinalsFieldData#getOrdinalMap with more targeted methods. #2

Closed
wants to merge 1 commit into from

Conversation

jtibshirani
Copy link
Owner

This commit stops exposing OrdinalMap in favor of a more tightly scoped method
to retrieve global ordinals. It also introduces a new interface
ConcreteOrdinalsFieldData so that HasChildQueryBuilder is able to get
ahold of the concrete OrdinalMap.

This commit stops exposing OrdinalMap in favor of a more tightly scoped method
to retrieve global ordinals. It also introduces a new interface
ConcreteOrdinalsFieldData so that HasChildQueryBuilder is able to get
ahold of the concrete OrdinalMap.
@jtibshirani jtibshirani force-pushed the ordinals-field-data branch from 3f1e487 to f922626 Compare April 12, 2019 01:35

import org.apache.lucene.index.OrdinalMap;

public interface ConcreteOrdinalsFieldData extends IndexOrdinalsFieldData {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a good name, which suggests the design may not make sense :)

@jtibshirani jtibshirani deleted the ordinals-field-data branch April 16, 2019 16:51
jtibshirani pushed a commit that referenced this pull request May 12, 2020
* * StartsWith is case sensitive aware
* Added case sensitivity to EQL configuration
* case_sensitive parameter can be specified when running queries (default
is case insensitive)
* Added STARTS_WITH function to SQL as well

* Add case sensitive aware queryfolder tests

* Address reviews

* Address review #2
jtibshirani pushed a commit that referenced this pull request Mar 11, 2021
…astic#69765)

Previously we did not resolve the attributes recursively which meant that if a field or expression was re-aliased multiple times (through multiple levels of subqueries), the aliases were only resolved one level down. This led to failed query translation because `ReferenceAttribute`s were pointing to non-existing attributes during query translation.

For example the query

```sql
SELECT i AS j FROM ( SELECT int AS i FROM test) ORDER BY j
```

failed during translation because the `OrderBy` resolved the `j` ReferenceAttribute to another `i` ReferenceAttribute that was later removed by an Optimization:

```
OrderBy[[Order[j{r}#4,ASC,LAST]]]                                             ! OrderBy[[Order[i{r}#2,ASC,LAST]]]
\_Project[[j]]                                                                = \_Project[[j]]
  \_Project[[i]]                                                              !   \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..]
    \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] ! 
```

By resolving the `Attributes` recursively both `j{r}` and `i{r}` will resolve to `test.int{f}` above:

```
OrderBy[[Order[test.int{f}elastic#22,ASC,LAST]]]                                     = OrderBy[[Order[test.int{f}elastic#22,ASC,LAST]]]
\_Project[[j]]                                                                = \_Project[[j]]
  \_Project[[i]]                                                              !   \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..]
    \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] ! 
 ```

The scope of recursive resolution depends on how the `AttributeMap` is constructed and populated.

Fixes elastic#67237
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant