Skip to content

[BEAM-5072][SQL]Disable SortRemoveRule and Remove RelCollationTraitDef configuration in BeamQueryPlanner#6139

Merged
apilloud merged 1 commit intoapache:masterfrom
amaliujia:rui_wang-remove_RelCollationTraitDef
Aug 6, 2018
Merged

[BEAM-5072][SQL]Disable SortRemoveRule and Remove RelCollationTraitDef configuration in BeamQueryPlanner#6139
apilloud merged 1 commit intoapache:masterfrom
amaliujia:rui_wang-remove_RelCollationTraitDef

Conversation

@amaliujia
Copy link
Contributor

@amaliujia amaliujia commented Aug 3, 2018

Right now we configure RelCollationTraitDef in BeamQueryPlanner but seems like we don't use/implement it. However this configuration causes SortRemoveRule to apply sort to SortRel's input and then drop the SorRel (which does not really sort the SortRel's input).

Remove RelCollationTraitDefand disable SortRemoveRule for now.

If don't remove CollationTrait, one example is:

INFO: SQLPlan>
BeamIOSinkRel(table=[[beam, ORDER_ID_TABLE]], operation=[INSERT], flattened=[true])
  LogicalSort(sort0=[$0], dir0=[ASC])
    LogicalProject(order_id=[$0])
      BeamIOSourceRel(table=[[beam, ORDER_DETAILS]])

Aug 03, 2018 1:07:24 PM org.apache.beam.sdk.extensions.sql.impl.BeamQueryPlanner convertToBeamRel
INFO: BEAMPlan>
BeamIOSinkRel(table=[[beam, ORDER_ID_TABLE]], operation=[INSERT], flattened=[true])
  BeamCalcRel(expr#0..3=[{inputs}], order_id=[$t0])
    BeamIOSourceRel(table=[[beam, ORDER_DETAILS]])

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

It will help us expedite review of your Pull Request if you tag someone (e.g. @username) to look at it.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
--- --- --- ---

@amaliujia
Copy link
Contributor Author

R: @apilloud @akedin

Also created JIRA https://issues.apache.org/jira/browse/BEAM-5073 to track enabling SortRemoveRule.

@amaliujia
Copy link
Contributor Author

@amaliujia amaliujia force-pushed the rui_wang-remove_RelCollationTraitDef branch from e1f6aea to c10dde9 Compare August 3, 2018 22:39
@apilloud
Copy link
Member

apilloud commented Aug 3, 2018

What is here so far looks good, you also need to remove it from the JDBC planner here:

Planner gives you getRelTraitDefs, clearRelTraitDefs, and addRelTraitDef, but nothing to remove a single trait. You'll have to get the list, clear them all, then add back the other ones.

@apilloud apilloud self-requested a review August 3, 2018 22:43

final ImmutableList<RelTraitDef> traitDefs =
ImmutableList.of(ConventionTraitDef.INSTANCE, RelCollationTraitDef.INSTANCE);
final ImmutableList<RelTraitDef> traitDefs = ImmutableList.of(ConventionTraitDef.INSTANCE);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only valid for BeamSqlEnv path. Does the JDBC path apply the CollationTrait?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amaliujia amaliujia force-pushed the rui_wang-remove_RelCollationTraitDef branch from c10dde9 to 5ab3ed8 Compare August 3, 2018 23:02
@amaliujia
Copy link
Contributor Author

I also removed/commented SortRemoveRule because it does nothing without CollationTrait

@amaliujia
Copy link
Contributor Author

Green

@akedin
Copy link
Contributor

akedin commented Aug 3, 2018

@amaliujia amaliujia changed the title [BEAM-5072] Remove RelCollationTraitDef configuration in BeamQueryPlanner [BEAM-5072][SQL]Remove RelCollationTraitDef configuration in BeamQueryPlanner Aug 5, 2018
Copy link
Member

@apilloud apilloud left a comment

Choose a reason for hiding this comment

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

LGTM.

@apilloud apilloud merged commit 6429412 into apache:master Aug 6, 2018
@amaliujia amaliujia deleted the rui_wang-remove_RelCollationTraitDef branch August 6, 2018 20:30
@amaliujia amaliujia changed the title [BEAM-5072][SQL]Remove RelCollationTraitDef configuration in BeamQueryPlanner [BEAM-5072][SQL]Disable SortRemoveRule and Remove RelCollationTraitDef configuration in BeamQueryPlanner Jul 15, 2019
@amaliujia amaliujia mentioned this pull request Jul 15, 2019
3 tasks
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.

3 participants