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

Add argument-less builder methods for boolean values #12476

Merged
merged 1 commit into from
May 23, 2022

Conversation

kasiafi
Copy link
Member

@kasiafi kasiafi commented May 19, 2022

The TableArgumentSpecification builder is part of SPI for declaring
Table Functions. It assumes default values for boolean table argument
properties, and allows to change them using argument-less methods.
It is meant to provide the easiest and most intuitive way of declaring
Table Functions.

This is a SPI change.

No documentation needed.

@cla-bot cla-bot bot added the cla-signed label May 19, 2022
@kasiafi
Copy link
Member Author

kasiafi commented May 19, 2022

@findepi how do I deal with SPI backwards compatibility assertions?

but could not find the following element(s):
2022-05-19T11:14:04.2842320Z  <["Method: public io.trino.spi.ptf.TableArgumentSpecification$Builder io.trino.spi.ptf.TableArgumentSpecification$Builder.rowSemantics(boolean)",
2022-05-19T11:14:04.2842672Z     "Method: public io.trino.spi.ptf.TableArgumentSpecification$Builder io.trino.spi.ptf.TableArgumentSpecification$Builder.pruneWhenEmpty(boolean)",
2022-05-19T11:14:04.2843027Z     "Method: public io.trino.spi.ptf.TableArgumentSpecification$Builder io.trino.spi.ptf.TableArgumentSpecification$Builder.passThroughColumns(boolean)"]>

Should I deprecate the old methods instead of remove? I'd rather remove them right away, since this is a new feature with no usage so far.

@findepi
Copy link
Member

findepi commented May 19, 2022

Should I deprecate the old methods instead of remove?

i think so

I'd rather remove them right away, since this is a new feature with no usage so far.

You'd need to add exclusion to io.trino.spi.TestSpiBackwardCompatibility#BACKWARD_INCOMPATIBLE_CHANGES

@findepi
Copy link
Member

findepi commented May 19, 2022

i'm fine either way

The TableArgumentSpecification builder is part of SPI for declaring
Table Functions. It assumes default values for boolean table argument
properties. It is meant to provide the easiest and most intuitive way
of declaring TableFunctions.
@martint
Copy link
Member

martint commented May 20, 2022

Should I deprecate the old methods instead of remove? I'd rather remove them right away, since this is a new feature with no usage so far.

The feature is experimental and it’s not being used by anyone, so no point in trying to keep it compatible. Let’s just remove it.

I expect we’ll have to make a few more changes as we continue implementing the rest of the functionality to be able to execute the functions.

@kasiafi
Copy link
Member Author

kasiafi commented May 23, 2022

@findepi I removed the old methods and added exclusion. Is it ready to go?

@kasiafi kasiafi merged commit d292ded into trinodb:master May 23, 2022
@github-actions github-actions bot added this to the 382 milestone May 23, 2022
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.

3 participants