Support uses of BACK that cause correlated references: setup decorrelation handling #251

knassre-bodo · 2025-02-06T21:31:10Z

Child PR of #269 that is part of addressing #141. Adds the handling of de-correlation as a post-processing step of the hybrid tree before relational conversion starts. Handles the singular & aggregate cases as described in the issue, and handles the semi-singular and semi-aggregate cases in the same manner as their non-semi equivalents (for now), saving the optimized variants for another day.

These remaining still need to be addressed in the next follow up:

TPC-H Q22: error during relational conversion that likely means a bug happened during hybrid conversion or hybrid decorrelation
Correl 15: error during decorrelation causing over-excessive column pruning
Correl 18: tbd bug in decorrelation
Correl 19: bug in name resolution handling that causes the wrong term to be used when certain property names collide

Co-authored-by: Hadia Ahmed <[email protected]>

Revision 2 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 3 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 4 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 5 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 6 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 7 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 8 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 9 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 10 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 11 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 12 Co-authored-by: Hadia Ahmed <[email protected]>

Revision 13 Co-authored-by: Hadia Ahmed <[email protected]>

knassre-bodo · 2025-02-10T22:06:36Z

pydough/conversion/hybrid_tree.py

+        if levels == 0:
+            return self


This is just a useful trick to have rather than having to case on this behavior elsewhere, so x.shift_back(0) returns x.

knassre-bodo · 2025-02-10T22:07:04Z

pydough/conversion/hybrid_tree.py

@@ -895,6 +897,7 @@ def __init__(
        self._is_connection_root: bool = is_connection_root
        self._agg_keys: list[HybridExpr] | None = None
        self._join_keys: list[tuple[HybridExpr, HybridExpr]] | None = None
+        self._correlated_children: set[int] = set()


This set of indices indicates which of the elements in _children contain a correlated reference specifically to self. This is important because only those children should have the de-correlation procedure run on them, and only if they are of a certain type.

knassre-bodo · 2025-02-10T22:09:44Z

pydough/conversion/hybrid_tree.py

-                case HybridRefExpr():
-                    parent_result = HybridBackRefExpr(
-                        parent_result.expr.name, 1, parent_result.typ
-                    )


This weird extra-shifting trick is no longer needed, and was actually incorrect.

knassre-bodo · 2025-02-10T22:10:44Z

pydough/conversion/hybrid_tree.py

+        if not isinstance(parent_result, HybridCorrelExpr):
+            parent_tree.correlated_children.add(len(parent_tree.children))


This is how we know which children of the hybrid tree are correlated. If parent_result is a correl expr, it means that the correlated reference is actually to the tree containing parent_tree.

knassre-bodo · 2025-02-10T22:12:34Z

tests/test_plan_refsols/correl_1.txt

@@ -1,8 +1,10 @@
 ROOT(columns=[('name', name), ('n_prefix_nations', n_prefix_nations)], orderings=[(ordering_1):asc_first])
 PROJECT(columns={'n_prefix_nations': n_prefix_nations, 'name': name, 'ordering_1': name})
  PROJECT(columns={'n_prefix_nations': DEFAULT_TO(agg_0, 0:int64), 'name': name})
-   JOIN(conditions=[t0.key == t1.region_key], types=['left'], columns={'agg_0': t1.agg_0, 'name': t0.name}, correl_name='corr1')
+   JOIN(conditions=[t0.key == t1.key], types=['left'], columns={'agg_0': t1.agg_0, 'name': t0.name})


This is a "simple" example of how the de-correaltion works.

The left subtree (region) became a part of the right subtree (nation) before the correlated reference (the filter) occurred. This results in selecting region twice, but that's an efficiency problem for another day.

The join is now on the uniquness keys of the left subtree: the region's .key property.

amullerbodo

general thoughts about hashes and equality.

amullerbodo · 2025-02-17T00:00:31Z

pydough/conversion/hybrid_decorrelater.py

+                ):
+                    # These patterns do not require decorrelation since they
+                    # are supported via correlated SEMI/ANTI joins.
+                    continue


do we need a default "case _:" here?

No because these are exhaustive with regards to the enum. If any new case is generated, mypy will throw an exception because these branches are no longer exhaustive. So here, we can rely on mypy tooling to ensure quality.

amullerbodo · 2025-02-17T00:06:04Z

pydough/pydough_operators/base_operator.py

+        return self.equals(other)
+
+    def __hash__(self) -> int:
+        return hash(repr(self))


any other way we could generate this hash? If someone changes repr inadvertently somewhere else it could break things. Not sure it is something we should be worried about at this stage.

I can't think of a better way off the top of my head since the best way to hash would be in a manner that depends on the name of the function/operator, which is always inside of its repr string (but each class handles that naming differently).

pydough/conversion/hybrid_decorrelater.py

vineetg3

Few Typos, but LGTM!

Resolves #141. See issue for more details. Child PRs deal with the components of the issue, ancillary changes, and additional bugfixes: - #232 - #234 - #251 - #254 - #272

knassre-bodo and others added 30 commits January 13, 2025 11:59

Starting function list documentation

deeb914

Adding datetime functions and bad boolean tests

5d6c513

Merge branch 'main' into kian/function_docs

170492e

Adding remaining functions including agg/window

8b8c098

Adding toc

cb83f1f

Adding toc

ef39e6e

Fixing typo [RUN CI]

3c79543

Started DSL documentation

3db13d1

Adding calc, contextless, and back

8dc17b3

Added TOC and TODOs

044c217

Added TOC and TODOs

0252cee

Changing highlighting

98c722b

Merge branch 'kian/function_docs' into kian/dsl_docs

aa86d31

Addded more examples

b03a5ec

Added WHERE, ORDER_BY, and TOP_K documentation

b569bfb

Fixing typo

7607fea

Added extra example

215735b

Update pydough/unqualified/unqualified_node.py

0b8e1fb

Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

a5190f8

Revision 2 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

a0d43f3

Revision 3 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

24c1e4a

Revision 4 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

8009e05

Revision 5 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

5d87b59

Revision 6 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

a3ee536

Revision 7 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

9945752

Revision 8 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

4c6aa79

Revision 9 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

e60dcc3

Revision 10 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

a23c395

Revision 11 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

2515683

Revision 12 Co-authored-by: Hadia Ahmed <[email protected]>

Update documentation/functions.md

d34c64c

Revision 13 Co-authored-by: Hadia Ahmed <[email protected]>

knassre-bodo added 4 commits February 10, 2025 13:37

Pulling up downstream test changes

ad61d27

Merge branch 'kian/corrleated_backref' into kian/correlated_backref_2

07d771d

Resolving conflicts

ba095ec

Cleanup

7ef0476

knassre-bodo marked this pull request as ready for review February 10, 2025 20:18

knassre-bodo requested review from hadia206 and vineetg3 February 10, 2025 22:04

knassre-bodo commented Feb 10, 2025

View reviewed changes

Removing dead code

c83b67e

knassre-bodo commented Feb 10, 2025

View reviewed changes

knassre-bodo requested a review from amullerbodo February 11, 2025 20:38

amullerbodo reviewed Feb 17, 2025

View reviewed changes

Revisions [RUN CI]

fdbf909

knassre-bodo mentioned this pull request Feb 17, 2025

Add support for uses of BACK that cause correlated references #269

Merged

knassre-bodo added 4 commits February 17, 2025 15:24

Merge branch 'kian/correlation_omnibus' into kian/corrleated_backref

ec77c0d

Revisions

a5531e3

Merge branch 'kian/corrleated_backref' into kian/correlated_backref_2

4becac1

Resolve conflicts

c4af826

Base automatically changed from kian/correlated_backref_2 to kian/correlation_omnibus February 17, 2025 20:28

knassre-bodo added 2 commits February 17, 2025 15:28

Merge branch 'kian/correlated_backref_2' into kian/correlated_backref_3

f44ba0e

Merge branch 'kian/correlation_omnibus' into kian/correlated_backref_3

11ac86d

vineetg3 reviewed Feb 18, 2025

View reviewed changes

pydough/conversion/hybrid_decorrelater.py Outdated Show resolved Hide resolved

vineetg3 approved these changes Feb 18, 2025

View reviewed changes

Fixing typos

2105bc8

knassre-bodo merged commit b304c29 into kian/correlation_omnibus Feb 18, 2025
3 checks passed

knassre-bodo deleted the kian/correlated_backref_3 branch February 18, 2025 20:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support uses of BACK that cause correlated references: setup decorrelation handling #251

Support uses of BACK that cause correlated references: setup decorrelation handling #251

knassre-bodo commented Feb 6, 2025 •

edited

Loading

knassre-bodo Feb 10, 2025

knassre-bodo Feb 10, 2025 •

edited

Loading

knassre-bodo Feb 10, 2025

knassre-bodo Feb 10, 2025

knassre-bodo Feb 10, 2025

amullerbodo left a comment

amullerbodo Feb 17, 2025

knassre-bodo Feb 17, 2025 •

edited

Loading

amullerbodo Feb 17, 2025

knassre-bodo Feb 17, 2025 •

edited

Loading

vineetg3 left a comment

		if not isinstance(parent_result, HybridCorrelExpr):
		parent_tree.correlated_children.add(len(parent_tree.children))

Support uses of BACK that cause correlated references: setup decorrelation handling #251

Support uses of BACK that cause correlated references: setup decorrelation handling #251

Conversation

knassre-bodo commented Feb 6, 2025 • edited Loading

knassre-bodo Feb 10, 2025

Choose a reason for hiding this comment

knassre-bodo Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

knassre-bodo Feb 10, 2025

Choose a reason for hiding this comment

knassre-bodo Feb 10, 2025

Choose a reason for hiding this comment

knassre-bodo Feb 10, 2025

Choose a reason for hiding this comment

amullerbodo left a comment

Choose a reason for hiding this comment

amullerbodo Feb 17, 2025

Choose a reason for hiding this comment

knassre-bodo Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

amullerbodo Feb 17, 2025

Choose a reason for hiding this comment

knassre-bodo Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

vineetg3 left a comment

Choose a reason for hiding this comment

knassre-bodo commented Feb 6, 2025 •

edited

Loading

knassre-bodo Feb 10, 2025 •

edited

Loading

knassre-bodo Feb 17, 2025 •

edited

Loading

knassre-bodo Feb 17, 2025 •

edited

Loading