-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[Calcite 3558] Fixing indexing variable for the column reordering loop #1624
base: main
Are you sure you want to change the base?
Conversation
Sync with the master branch
Sync with the Calcite master branch (5/29/2019)
sync with calcite master
Sync with the remote master
Branch 1.21
latest master 12/3/2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please modify the commit message. You can refer to this doc[1].
@@ -243,6 +243,7 @@ protected RelNode appendSearchString( | |||
field = topFields.get(i); | |||
} | |||
newFields.add(field); | |||
i++; | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add a test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And change the commit message to the problem being fixed, not the code change you made to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update test cases and commit message.
49cb002
to
8768a23
Compare
8a5cf83
to
cf7f71b
Compare
Just simply put i++ in the loop to make the loop work properly.