-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Refactor table function analysis #12407
Conversation
@@ -1488,7 +1488,7 @@ protected Scope visitTableFunctionInvocation(TableFunctionInvocation node, Optio | |||
ConnectorTransactionHandle transactionHandle = transactionManager.getConnectorTransaction( | |||
session.getRequiredTransactionId(), | |||
getRequiredCatalogHandle(metadata, session, node, catalogName.getCatalogName())); | |||
ConnectorTableFunction.Analysis functionAnalysis = function.analyze(session.toConnectorSession(catalogName), transactionHandle, passedArguments); | |||
io.trino.spi.ptf.Analysis functionAnalysis = function.analyze(session.toConnectorSession(catalogName), transactionHandle, passedArguments); |
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.
rename Analysis so it can be imported here
* gathered at analysis time. Typically, these are the values of the constant arguments, and results | ||
* of pre-processing arguments. | ||
*/ | ||
public class Analysis |
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.
final
(this is SPI, so let's be defensive)
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.
%
The `analyze()` method of a `ConnectorTableFunction` must be implemented by every subclass. It is now marked as abstract.
c2ef5be
to
131dc44
Compare
This is a refactor around table function Analysis.
This is a SPI change. No release notes needed if this change is merged before the upcoming release (the main change has not been yet released)
This change has to be taken into account in the documentation (#12338)