[CALCITE-6340] RelBuilder drops traits when aggregating over duplicate projected fields

[jduo]

Calculate the post-pruning RelTraitSet on the projection using TraitSet#apply(Mapping)

[asolimando]

@jduo overall LGTM but there are a couple of changes I'd like you to make before getting this in, please take a look

[asolimando]

Few modifications as described in the unresolved comments

[asolimando]

@jduo, I have made some modifications to the PR in this commit, can you take a look?

I have added a conditional check on the results based on the status of the ticket you filed (as it's customary in Calcite), and I have also refined some tests and added few more

EDIT: finally it seems that all traits are dropped by this bug, so I suggest to rename the jira ticket as "[CALCITE-6340] RelBuilder drops traits when aggregating over duplicate projected fields" and update it in the final commit (when you will be squashing after getting approval) and wherever it is referenced in the code

[jduo]

@jduo, I have made some modifications to the PR in this commit, can you take a look?

I have added a conditional check on the results based on the status of the ticket you filed (as it's customary in Calcite), and I have also refined some tests and added few more

EDIT: finally it seems that all traits are dropped by this bug, so I suggest to rename the jira ticket as "[CALCITE-6340] RelBuilder drops traits when aggregating over duplicate projected fields" and update it in the final commit (when you will be squashing after getting approval) and wherever it is referenced in the code

I think the changes proposed are mostly OK, but the Bug entry should be for the new ticket (6391) instead of this one (6340).

[asolimando]

Few more minor changes and for me we are good to go.

Please don't force-push until the review process is on-going as it makes incremental reviewing more difficult.

[jduo]

Few more minor changes and for me we are good to go.

Please don't force-push until the review process is on-going as it makes incremental reviewing more difficult.

In this case I force-pushed because there was a merge conflict against the head. Would it be preferred if I waited for the review to finish before dealing with these?

[asolimando]

Few more minor changes and for me we are good to go.
Please don't force-push until the review process is on-going as it makes incremental reviewing more difficult.

In this case I force-pushed because there was a merge conflict against the head. Would it be preferred if I waited for the review to finish before dealing with these?

Unless for cases in which this is breaking CI then it's indeed preferable for the review process to be over.

[asolimando]

LGTM, thanks @jduo for taking care of the comments, can you squash the commits into a single one and make sure the commit message matches exactly the title of the Jira ticket preceded by [CALCITE-6340] (you can check the guidelines here if you are in doubt).

After that, I will wait for a couple more days in case someone has further comments, then I will merge.

[jduo]

LGTM, thanks @jduo for taking care of the comments, can you squash the commits into a single one and make sure the commit message matches exactly the title of the Jira ticket preceded by [CALCITE-6340] (you can check the guidelines here if you are in doubt).

After that, I will wait for a couple more days in case someone has further comments, then I will merge.

Thanks @asolimando . I've squashed and updated the title.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud