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.

Sign up for GitHub

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

Jump to bottom

Multiple JexlNode rebuilding visitors are not preserving proper parentage #880

Open
ivakegg opened this issue Jul 31, 2020 · 0 comments
Open

Multiple JexlNode rebuilding visitors are not preserving proper parentage #880

ivakegg opened this issue Jul 31, 2020 · 0 comments
Assignees
@lbschanno

Comments

@ivakegg
Copy link
Collaborator

ivakegg commented Jul 31, 2020

There is a new validateLineage method being added to JexlASTHelper which attempts to validate that for a given Jexl tree:

  1. every child node has a non-null parent equal to the parent who listed that node as a child
  2. all children are non-null
    Using this method we can determine where in the planning process (DefaultQueryPlanner) that we are creating invalid trees. Those steps/rebuilding visitors need to be fixed.

One known area is when when getQueryPropertySource is called on the QueryPropertyMarker.

Once everything is fixed, we should be able to call validateLineage before and after every step in the DefaultQueryPlanner with no errors.
@lbschanno lbschanno self-assigned this Sep 8, 2020
@lbschanno lbschanno mentioned this issue Sep 26, 2020
Closed
This was referenced Oct 20, 2020
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
jwomeara pushed a commit that referenced this issue Oct 21, 2020
@lbschanno
Verify lineage for RegexFunctionVisitor (#964)
511861d 
Add asserts to tests for RegexFunctionVisitor to verify that it returns
a query tree with valid lineage.

Part of #880.
@lbschanno lbschanno mentioned this issue Oct 21, 2020
Merged
jwomeara added a commit that referenced this issue Oct 30, 2020
@lbschanno @jwomeara
Verify lineage for IsNotNullIntentVisitor (#966)
90ad7f3 
Verify that IsNotNullIntentVisitor returns a query tree with a valid
lineage, and add unit tests to assert this.

Part of #880.

Co-authored-by: Whitney O'Meara <jwomeara@users.noreply.github.com>
jwomeara added a commit that referenced this issue Oct 30, 2020
@lbschanno @jwomeara
Verify lineage for GeoWavePruningVisitor (#961)
616179f 
Add asserts to tests for GeoWavePruningVisitor to verify that it returns
a query tree with valid lineage.

Part of #880.

Co-authored-by: Whitney O'Meara <jwomeara@users.noreply.github.com>
jwomeara added a commit that referenced this issue Oct 30, 2020
@lbschanno @jwomeara
Verify lineage for FixNegativeNumbersVisitor (#960)
91871cc 
Add unit tests to verify that FixNegativeNumbersVisitor returns a query
tree with valid lineage.

Part of #880.

Co-authored-by: Whitney O'Meara <jwomeara@users.noreply.github.com>
jwomeara added a commit that referenced this issue Oct 30, 2020
@lbschanno @jwomeara
Verify lineage for DateIndexCleanupVisitor (#957)
0d0946a 
Add an assert to tests for DateIndexCleanupVisitor to verify that it
returns a query tree with a valid lineage.

Part of #880.

Co-authored-by: Whitney O'Meara <jwomeara@users.noreply.github.com>
jwomeara added a commit that referenced this issue Oct 30, 2020
@lbschanno @jwomeara
Fix lineage for BooleanOptimizationRebuildingVisitor (#955)
2ade322 
BooleanOptimizationRebuildingVisitor does not return a query tree with a
valid lineage. Ensure that lineage is properly established and an assert
for this in related tests.

Related to #880.

Co-authored-by: Whitney O'Meara <jwomeara@users.noreply.github.com>
ivakegg pushed a commit that referenced this issue Oct 30, 2020
@lbschanno @ivakegg
Fix lineage for FacetCheck
73086a8 
FacetCheck does not return a query tree with a valid lineage. Ensure
that lineage is properly established and add unit tests to assert this.

Part of #880.
jwomeara added a commit that referenced this issue Nov 3, 2020
@lbschanno @jwomeara
Verify lineage for TreeFlatteningRebuildingVisitor (#959)
bc53626 
* Verify lineage for TreeFlatteningRebuildingVisitor

Add an assert to tests for TreeFlatteningRebuildingVisitor to verify
that it returns a query with valid lineage.

Part of #880.

* Restore return value for copyTree method

Co-authored-by: Whitney O'Meara <jwomeara@users.noreply.github.com>
This was referenced Nov 5, 2020
Merged
Merged
@lbschanno lbschanno mentioned this issue Nov 19, 2020
Merged
ivakegg pushed a commit that referenced this issue Dec 11, 2020
@lbschanno
re #880: Fix lineage for AllTermsIndexedVisitor (#954)
c4fb7f3 
* Fix lineage for AllTermsIndexedVisitor

AllTermsIndexedVisitor did not return a query tree with a valid lineage.
Ensure that lineage is properly established and add an assert for this
in related tests.
ivakegg pushed a commit that referenced this issue Dec 11, 2020
@lbschanno
re #880, #968: Fix lineage for ExpandCompositeTerms (#995)
cad4870 
* Ensure that ExpandCompositeTerms returns a JEXL tree with a valid
lineage, and ensure that it does not modify the original input JEXL
tree.
ivakegg pushed a commit that referenced this issue Dec 11, 2020
@lbschanno
re #880: Verify lineage for ExpandMultinormalizedTerms (#958)
fb30e94 
* Add an assert to tests for ExpandMultinormalizedTerms to verify that it
returns a query tree with valid lineage.
* Dedupe and optimize JexlASTHelper.isDelayedPredicate
* Revert isDelayedPredicate changes
* Avoid multiple recursions
* Move isDelayedPredicate method to QueryPropertyMarkerVisitor
* Remove obsolete constant
ivakegg pushed a commit that referenced this issue Dec 18, 2020
@lbschanno
re #880, #968: Verify FunctionIndexQueryExpansionVisitor does not mod…
ddc64b4 
…ify input tree (#998)

* Verify that FunctionIndexQueryExpansionVisitor does not modify the
original input JEXL tree, or invalidate its lineage. In addition, verify
that the visitor returns a JEXL tree with a valid lineage.
ivakegg pushed a commit that referenced this issue Mar 5, 2021
@lbschanno
Fix lineage for UniqueExpressionTermsVisitor (#994)
f2af3e4 
* Fix lineage for UniqueExpressionTermsVisitor to ensure that it returns a JEXL tree with a valid lineage, and does not modify the original input tree.
Part of #880 and #968
@apmoriarty apmoriarty mentioned this issue Jul 28, 2021
Open
This was referenced Aug 24, 2021
Merged
Merged
Merged
Merged
apmoriarty pushed a commit that referenced this issue Aug 27, 2021
@lbschanno
Verify lineage for TreeWrappingRebuildingVisitor (#1262)
7361524 
Write tests for TreeWrappingRebuildingVisitor to verify that it returns
a query with valid lineage.

Part of #880.
@lbschanno lbschanno mentioned this issue Aug 27, 2021
Merged
ivakegg pushed a commit that referenced this issue Sep 16, 2021
@lbschanno
Verify lineage for QueryOptionsFromQueryVisitor (#1265)
67126f9 
Part of #880

* Write tests for QueryOptionsFromQueryVisitor to verify that it returns a
query with a valid lineage. Additionally, update the documentation, and
ensure that any AND/OR nodes with subsequently deleted children are
cleaned up.
ivakegg pushed a commit that referenced this issue Sep 17, 2021
@lbschanno
Fix lineage for FixUnindexedNumericTerms (#1266)
29b2b7d 
FixUnindexedNumericTerms does not return a query tree with a valid
lineage. Ensure that lineage is properly established, and add unit tests
to verify this.

Part of #880.
ivakegg pushed a commit that referenced this issue Sep 17, 2021
@lbschanno
Fix lineage for FieldToFieldComparisonVisitor (#1267)
c64d411 
FieldToFieldComparisonVisitor does not return a query with a valid
lineage. Ensure that lineage is properly established, and add unit tests
to verify this.

Part of #880.
ivakegg pushed a commit that referenced this issue Sep 17, 2021
@lbschanno
Verify lineage for NodeTransformVisitor (#1268)
9b5f39f 
Modify tests for NodeTransformVisitor to viery that it returns a query
with a valid lineage.

Part of #880.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lbschanno lbschanno

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ivakegg @lbschanno