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

Refactor stivt code #791

Merged
merged 4 commits into from
Sep 20, 2022
Merged

Refactor stivt code #791

merged 4 commits into from
Sep 20, 2022

Conversation

lziq
Copy link
Contributor

@lziq lziq commented Sep 20, 2022

This PR aims to refactor code style of operators handling in StaticTypeInferenceVisitorTransform.kt for better readability. There is no behavior change in this PR

##Changes:

  1. Now the code of operators handling are structurally classified into the following 4 categories:
  • Unary operators: NOT, POS & NEG
  • Logical NAry operators: AND & OR
  • Common NAry operators: PLUS, MINUS, TIMES, DIVIDE, MODULO, CONCAT, EQ, NE, LT, LTE, GT&GTE`
  • Other special NAry operators: LIKE, BETWEEN & IN
    Each category has similar coding structure and calls similar helper methods. e.g. NAry Arithmetic operators & comparator operators now both call computeReturnTypeForNAry

@@ -269,64 +271,36 @@ internal class StaticTypeInferenceVisitorTransform(
return node.withStaticType(foundType)
}

private fun transformNAry(
Copy link
Member

@alancai98 alancai98 Sep 20, 2022

Choose a reason for hiding this comment

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

(Something to consider for future PRs) The diff for this PR is pretty hard to read because there's both refactoring of logic within functions and rearrangement of function order. One way to make the diff somewhat easier for reviewers to review could be to have two distinct commits: 1. performs logic changes/inner function refactoring. 2. rearranges the functions

@lziq
Copy link
Contributor Author

lziq commented Sep 20, 2022

thx

@lziq lziq merged commit 58dd2cb into main Sep 20, 2022
@lziq lziq deleted the refactor-STIVT-code branch September 20, 2022 22:23
lziq added a commit that referenced this pull request Oct 21, 2022
commit 05c1785
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Wed Oct 19 10:11:34 2022 -0700

    Applied comments in PR 822 (#836)

commit 8255a07
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Tue Oct 18 14:15:15 2022 -0700

    Set next dev version (#846)

commit bc4dc47
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Tue Oct 18 13:13:53 2022 -0700

    Bumps partiql-isl-kotlin to 0.2.2 (#845)

commit 55de098
Author: Alan Cai <caialan@amazon.com>
Date:   Mon Oct 17 14:01:40 2022 -0700

    Update conformance test runner skip list with ported `pts` tests (#829)

commit d8bcf1f
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Mon Oct 17 09:15:21 2022 -0700

    v0.8.0 (#844)

    Creates 0.8.0 commit

commit b4b7e1f
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Fri Oct 14 16:52:49 2022 -0700

    Moves partiql-grammar to org.partiql.lang.syntax.antlr so we don't vend a separate jar (#843)

commit 28d960f
Author: yliuuuu <107505258+yliuuuu@users.noreply.github.com>
Date:   Thu Oct 13 16:07:57 2022 -0700

    fix subquery execution (#842)

commit 0b8d14a
Author: Arash Maymandi <27716912+am357@users.noreply.github.com>
Date:   Thu Oct 13 16:04:55 2022 -0700

    Final changes for REPLACE/UPSERT/INSERT EXCLUDED (#831)

    This commit includes the following:
    - Support for INSERT INTO tbl ... ON CONFLICT DO REPLACE EXCLUDED and REPLACE INTO tbl ... evaluation.
    - Support for INSERT INTO tbl ... ON CONFLICT DO UPDATE EXCLUDED logical planning.
       - The reason that evaluation is excluded is b/c the logic is for update evaluation requires implementing merge with existing values which makes it more completed; since it's not part of the related ask (attached issue) we defer the implementation for now
    - Adds parsing support for AS alias for UPSERT INTO and REPLACE INTO statements.

commit 13f622f
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Thu Oct 13 15:50:06 2022 -0700

    Cleans up SORT and UNPIVOT factories (#839)

commit 17acf86
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Thu Oct 13 15:47:41 2022 -0700

    Updates changelog (#840)

commit 376bc18
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Thu Oct 13 14:45:45 2022 -0700

    Adds support for aggregations and grouping to the planner (#821)

commit 8be7a2d
Author: Alan Cai <caialan@amazon.com>
Date:   Wed Oct 12 13:37:59 2022 -0700

    Update README.md following addition of partiql-tests git submodule (#838)

commit 2210658
Merge: 5715a88 9fdc92c
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Wed Oct 12 10:40:25 2022 -0700

    Merge pull request #797 from partiql/graph-match-expr

    Switched from MATCH-as-FROM-source to MATCH-as-expression

commit 9fdc92c
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Wed Oct 12 10:36:39 2022 -0700

    Cleanup

commit 0dfc9ec
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Tue Oct 11 14:09:36 2022 -0700

    Put back tests from the reverted commit,
    adjusting the success status as appropriate.

commit a7b42bf
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Tue Oct 11 10:58:32 2022 -0700

    Revert "Allow unparethesized MATCH in more places."

    This reverts commit af6f6c3.

    It's hard or impossible to match this grammar addition in the partiql-lang-rust edition of the grammar.

commit 5715a88
Author: Josh Pschorr <josh@pschorr.dev>
Date:   Fri Oct 7 13:41:00 2022 -0700

    Default sort & null spec in evaluator, not parser (#834)

commit af6f6c3
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Thu Oct 6 23:11:09 2022 -0700

    Allow unparethesized MATCH in more places.

commit 9b26e9c
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Thu Oct 6 16:00:58 2022 -0700

    Grammar tweaks to not require (and prohibit) parentheses around a pattern with commas.
    Instead, require commas around the whole MATCH expression in this case.

commit 1ce2669
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Tue Oct 4 22:49:22 2022 -0700

    A couple more parsing tests, involving a bare edge pattern -[]->.

commit e552b92
Merge: 02b07cb a9e5a4e
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Tue Oct 4 17:36:45 2022 -0700

    Merge branch 'main' into graph-match-expr.

    Resolved a conflict in
    	lang/src/org/partiql/lang/eval/physical/PhysicalPlanCompilerImpl.kt

commit a9e5a4e
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Tue Oct 4 15:32:00 2022 -0700

    Removed snapshot from version (#828)

commit a0209df
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Mon Oct 3 13:57:55 2022 -0700

    Adds PIVOT to PartiQLPlanner (#817)

commit 9a159a0
Author: yliuuuu <107505258+yliuuuu@users.noreply.github.com>
Date:   Mon Oct 3 10:45:01 2022 -0700

    Adds UNPIVOT operator to PartiQLPlanner (#815)

    * support `unpivot` operator in planner

commit 07f4a27
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Fri Sep 30 17:02:57 2022 -0400

    Adds SORT operator to planner (#793)

commit f80e015
Author: Alan Cai <caialan@amazon.com>
Date:   Wed Sep 28 14:26:35 2022 -0700

    Add Kotlin conformance test runner (#809)

commit 882d7d2
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Tue Sep 27 17:11:55 2022 -0700

    Remove state from planner (#813)

commit a871ba5
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Tue Sep 27 10:42:13 2022 -0700

    Moves partiql-isl-kotlin to a library of partiql-lang-kotlin (#805)

commit cd16bf3
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Mon Sep 26 15:38:08 2022 -0700

    Removed type names from ExprValueType (#808)

commit 2e19331
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Mon Sep 26 08:23:20 2022 -0700

    Renames PhysicalExprToThunkConverter as PhysicalPlanCompiler (#801)

commit 706acd9
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Fri Sep 23 10:03:51 2022 -0700

    Marked TypedOpBehavior.LEGACY as deprecated (#796)

commit 02b07cb
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Fri Sep 23 09:58:05 2022 -0700

    Lint fixes.

commit f29f894
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Thu Sep 22 21:16:56 2022 -0700

    Switched from MATCH-as-FROM-source to MATCH-as-expression.

commit 4b3ec94
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Sun Sep 18 15:34:08 2022 -0700

    Rename AST node graph_match_expr ~~> gpml_pattern, and matchExpr ~~> gpmlPattern in the grammar.

    The new name is a tad more appropriate for the node's contents
    and will be clashing less with the upcoming new expression form graph_match.

commit 58e908d
Author: Arash Maymandi <27716912+am357@users.noreply.github.com>
Date:   Thu Sep 22 15:32:20 2022 -0700

    Add experimental REPLACE/UPSERT INTO (#788)

    * Add experimental REPLACE/UPSERT INTO

    Adding the parsing capability for `REPLACE INTO` and `REPLACE INTO`.
    See the PR and associated Issue for more details.

commit 8bfaf20
Author: Arash Maymandi <27716912+am357@users.noreply.github.com>
Date:   Thu Sep 22 14:38:16 2022 -0700

    Add logical plan support for DO REPLACE EXCLUDED (#792)

commit 96de8e7
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Wed Sep 21 14:45:58 2022 -0700

    Adds projection alias support to ORDER BY clause (#740)

    * Adds projection alias support to ORDER BY clause

commit 001a99b
Author: Arash Maymandi <27716912+am357@users.noreply.github.com>
Date:   Wed Sep 21 12:59:42 2022 -0700

    Add an additional example for ExprFunction (#786)

    * Add an additional example for ExprFunction

    Adds an example for using a custom function for working with elements
    of lists and struct members.

commit 58dd2cb
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Tue Sep 20 15:23:10 2022 -0700

    Refactor stivt code (#791)

commit 74b6cf6
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Mon Sep 19 10:49:12 2022 -0700

    Splits large tests to allow parallelization and speedup in builds (#779)

commit 16ccbc1
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Fri Sep 16 09:52:18 2022 -0700

    Refactor STIVT Code Style (#782)
lziq added a commit that referenced this pull request Oct 21, 2022
commit ebbe6dc
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Thu Oct 20 14:01:00 2022 -0700

    Update ISL to SNAPSHOT (#851)

commit a298d93
Author: Arash Maymandi <27716912+am357@users.noreply.github.com>
Date:   Thu Oct 20 12:13:52 2022 -0700

    Add redaction for INSERT/UPSERT/REPLACE INTO << >> (#850)

    This commit enables statement redaction for INSERT, REPLACE, and UPSERT.

    See the following for more details:
    #849

commit fb4b29f
Merge: 05c1785 8f8d6d9
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Thu Oct 20 10:31:39 2022 -0700

    Merge pull request #847 from partiql/issue-616

    Makes SqlException.errorContext non-nullable, resolving Issue #616.

commit 8f8d6d9
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Wed Oct 19 11:32:54 2022 -0700

    Attempting to fix divergence in the git submodule test/partiql-tests.

commit 05c1785
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Wed Oct 19 10:11:34 2022 -0700

    Applied comments in PR 822 (#836)

commit 89dacc2
Merge: 7ab757c 8255a07
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Tue Oct 18 17:17:22 2022 -0700

    Merge branch 'main' into issue-616

commit 8255a07
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Tue Oct 18 14:15:15 2022 -0700

    Set next dev version (#846)

commit 7ab757c
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Tue Oct 18 13:14:43 2022 -0700

    Makes SqlException.errorContext non-nullable, resolving Issue #616.

commit bc4dc47
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Tue Oct 18 13:13:53 2022 -0700

    Bumps partiql-isl-kotlin to 0.2.2 (#845)

commit 55de098
Author: Alan Cai <caialan@amazon.com>
Date:   Mon Oct 17 14:01:40 2022 -0700

    Update conformance test runner skip list with ported `pts` tests (#829)

commit d8bcf1f
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Mon Oct 17 09:15:21 2022 -0700

    v0.8.0 (#844)

    Creates 0.8.0 commit

commit b4b7e1f
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Fri Oct 14 16:52:49 2022 -0700

    Moves partiql-grammar to org.partiql.lang.syntax.antlr so we don't vend a separate jar (#843)

commit 28d960f
Author: yliuuuu <107505258+yliuuuu@users.noreply.github.com>
Date:   Thu Oct 13 16:07:57 2022 -0700

    fix subquery execution (#842)

commit 0b8d14a
Author: Arash Maymandi <27716912+am357@users.noreply.github.com>
Date:   Thu Oct 13 16:04:55 2022 -0700

    Final changes for REPLACE/UPSERT/INSERT EXCLUDED (#831)

    This commit includes the following:
    - Support for INSERT INTO tbl ... ON CONFLICT DO REPLACE EXCLUDED and REPLACE INTO tbl ... evaluation.
    - Support for INSERT INTO tbl ... ON CONFLICT DO UPDATE EXCLUDED logical planning.
       - The reason that evaluation is excluded is b/c the logic is for update evaluation requires implementing merge with existing values which makes it more completed; since it's not part of the related ask (attached issue) we defer the implementation for now
    - Adds parsing support for AS alias for UPSERT INTO and REPLACE INTO statements.

commit 13f622f
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Thu Oct 13 15:50:06 2022 -0700

    Cleans up SORT and UNPIVOT factories (#839)

commit 17acf86
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Thu Oct 13 15:47:41 2022 -0700

    Updates changelog (#840)

commit 376bc18
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Thu Oct 13 14:45:45 2022 -0700

    Adds support for aggregations and grouping to the planner (#821)

commit 8be7a2d
Author: Alan Cai <caialan@amazon.com>
Date:   Wed Oct 12 13:37:59 2022 -0700

    Update README.md following addition of partiql-tests git submodule (#838)

commit 2210658
Merge: 5715a88 9fdc92c
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Wed Oct 12 10:40:25 2022 -0700

    Merge pull request #797 from partiql/graph-match-expr

    Switched from MATCH-as-FROM-source to MATCH-as-expression

commit 9fdc92c
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Wed Oct 12 10:36:39 2022 -0700

    Cleanup

commit 0dfc9ec
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Tue Oct 11 14:09:36 2022 -0700

    Put back tests from the reverted commit,
    adjusting the success status as appropriate.

commit a7b42bf
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Tue Oct 11 10:58:32 2022 -0700

    Revert "Allow unparethesized MATCH in more places."

    This reverts commit af6f6c3.

    It's hard or impossible to match this grammar addition in the partiql-lang-rust edition of the grammar.

commit 5715a88
Author: Josh Pschorr <josh@pschorr.dev>
Date:   Fri Oct 7 13:41:00 2022 -0700

    Default sort & null spec in evaluator, not parser (#834)

commit af6f6c3
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Thu Oct 6 23:11:09 2022 -0700

    Allow unparethesized MATCH in more places.

commit 9b26e9c
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Thu Oct 6 16:00:58 2022 -0700

    Grammar tweaks to not require (and prohibit) parentheses around a pattern with commas.
    Instead, require commas around the whole MATCH expression in this case.

commit 1ce2669
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Tue Oct 4 22:49:22 2022 -0700

    A couple more parsing tests, involving a bare edge pattern -[]->.

commit e552b92
Merge: 02b07cb a9e5a4e
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Tue Oct 4 17:36:45 2022 -0700

    Merge branch 'main' into graph-match-expr.

    Resolved a conflict in
    	lang/src/org/partiql/lang/eval/physical/PhysicalPlanCompilerImpl.kt

commit a9e5a4e
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Tue Oct 4 15:32:00 2022 -0700

    Removed snapshot from version (#828)

commit a0209df
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Mon Oct 3 13:57:55 2022 -0700

    Adds PIVOT to PartiQLPlanner (#817)

commit 9a159a0
Author: yliuuuu <107505258+yliuuuu@users.noreply.github.com>
Date:   Mon Oct 3 10:45:01 2022 -0700

    Adds UNPIVOT operator to PartiQLPlanner (#815)

    * support `unpivot` operator in planner

commit 07f4a27
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Fri Sep 30 17:02:57 2022 -0400

    Adds SORT operator to planner (#793)

commit f80e015
Author: Alan Cai <caialan@amazon.com>
Date:   Wed Sep 28 14:26:35 2022 -0700

    Add Kotlin conformance test runner (#809)

commit 882d7d2
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Tue Sep 27 17:11:55 2022 -0700

    Remove state from planner (#813)

commit a871ba5
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Tue Sep 27 10:42:13 2022 -0700

    Moves partiql-isl-kotlin to a library of partiql-lang-kotlin (#805)

commit cd16bf3
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Mon Sep 26 15:38:08 2022 -0700

    Removed type names from ExprValueType (#808)

commit 2e19331
Author: R. C. Howell <RCHowell@users.noreply.github.com>
Date:   Mon Sep 26 08:23:20 2022 -0700

    Renames PhysicalExprToThunkConverter as PhysicalPlanCompiler (#801)

commit 706acd9
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Fri Sep 23 10:03:51 2022 -0700

    Marked TypedOpBehavior.LEGACY as deprecated (#796)

commit 02b07cb
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Fri Sep 23 09:58:05 2022 -0700

    Lint fixes.

commit f29f894
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Thu Sep 22 21:16:56 2022 -0700

    Switched from MATCH-as-FROM-source to MATCH-as-expression.

commit 4b3ec94
Author: Vladimir Gapeyev <9939945+vgapeyev@users.noreply.github.com>
Date:   Sun Sep 18 15:34:08 2022 -0700

    Rename AST node graph_match_expr ~~> gpml_pattern, and matchExpr ~~> gpmlPattern in the grammar.

    The new name is a tad more appropriate for the node's contents
    and will be clashing less with the upcoming new expression form graph_match.

commit 58e908d
Author: Arash Maymandi <27716912+am357@users.noreply.github.com>
Date:   Thu Sep 22 15:32:20 2022 -0700

    Add experimental REPLACE/UPSERT INTO (#788)

    * Add experimental REPLACE/UPSERT INTO

    Adding the parsing capability for `REPLACE INTO` and `REPLACE INTO`.
    See the PR and associated Issue for more details.

commit 8bfaf20
Author: Arash Maymandi <27716912+am357@users.noreply.github.com>
Date:   Thu Sep 22 14:38:16 2022 -0700

    Add logical plan support for DO REPLACE EXCLUDED (#792)

commit 96de8e7
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Wed Sep 21 14:45:58 2022 -0700

    Adds projection alias support to ORDER BY clause (#740)

    * Adds projection alias support to ORDER BY clause

commit 001a99b
Author: Arash Maymandi <27716912+am357@users.noreply.github.com>
Date:   Wed Sep 21 12:59:42 2022 -0700

    Add an additional example for ExprFunction (#786)

    * Add an additional example for ExprFunction

    Adds an example for using a custom function for working with elements
    of lists and struct members.

commit 58dd2cb
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Tue Sep 20 15:23:10 2022 -0700

    Refactor stivt code (#791)

commit 74b6cf6
Author: John Ed Quinn <40360967+johnedquinn@users.noreply.github.com>
Date:   Mon Sep 19 10:49:12 2022 -0700

    Splits large tests to allow parallelization and speedup in builds (#779)

commit 16ccbc1
Author: lziq <67429381+lziq@users.noreply.github.com>
Date:   Fri Sep 16 09:52:18 2022 -0700

    Refactor STIVT Code Style (#782)
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.

2 participants