Skip to content

Fix bad parameter count in code generator when column uses double slots#15979

Merged
dain merged 2 commits intotrinodb:masterfrom
zhaner08:fix_double_slots_code_gen_issue
Mar 27, 2023
Merged

Fix bad parameter count in code generator when column uses double slots#15979
dain merged 2 commits intotrinodb:masterfrom
zhaner08:fix_double_slots_code_gen_issue

Conversation

@zhaner08
Copy link
Copy Markdown
Contributor

@zhaner08 zhaner08 commented Feb 5, 2023

Description

This change [/pull/7014] was trying to fix a bad parameter count issue when there are too many parameters for code generators, and this change makes sure when there are 100 or below number of parameters, BLOCK_POSITION will not be used to prevent issues. However, 100 is not enough especially when it comes to double slots type like Long. Double slots for long plus a nullable flag will cause the number of parameters surpass the max of 255 and causing query compiler error. This change further limits it down so double slots types with number of parameters between 85 to 100 will not throw error

Additional context and related issues

/pull/7014

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(X) Release notes are required, with the following suggested text:

# General
* Fix an compiler error when calling a function with a large of parameters. ({issue}`15979`)

@cla-bot cla-bot bot added the cla-signed label Feb 5, 2023
@zhaner08 zhaner08 requested a review from dain February 5, 2023 20:25
@zhaner08 zhaner08 requested a review from martint February 24, 2023 20:39
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I assume this was chosed becuse 256 / 3 = 85.3. There are other parameters that can be added, like session. I would make this 64 instead, which leaves plenty of space for extra stuff.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the number was calculated that way and I tested with the parameters counts around that threshold and all queries compiled correctly

Copy link
Copy Markdown
Contributor Author

@zhaner08 zhaner08 left a comment

Choose a reason for hiding this comment

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

Updated to 64 and rebased from latest

@zhaner08 zhaner08 force-pushed the fix_double_slots_code_gen_issue branch from 6b7960b to b08a3cc Compare March 14, 2023 15:46
@dain dain merged commit c033283 into trinodb:master Mar 27, 2023
@github-actions github-actions bot added this to the 411 milestone Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants