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
Process Request for routing rules #325
base: main
Are you sure you want to change the base?
Conversation
gateway-ha/src/main/java/io/trino/gateway/ha/config/HaGatewayConfiguration.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/config/HaGatewayConfiguration.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/config/ProcessedRequestConfig.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/router/ProcessedRequest.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/router/ProcessedRequest.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/router/ProcessedRequest.java
Outdated
Show resolved
Hide resolved
return new String(preparedStatement, UTF_8); | ||
} | ||
|
||
private void getTables(Node node, Set<QualifiedName> tables) |
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 add a unit test for this method.
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.
This method is tested by TestRoutingGroupSelector.testRequestProcessorQueryDetails
. For the SQL
SELECT x.*, y.*, z.* FROM catx.schemx.tblx a, schemy.tbly y, tblz z
with default catalog and schema of cat_default
and schem_"default
, The matched rule checks
processedRequest.tablesContains("cat_default.\"schem_\\\"default\".tblz")
&& processedRequest.tablesContains("cat_default.schemy.tbly")
&& processedRequest.tablesContains("catx.schemx.tblx")
&& processedRequest.getSchemas().contains("schemy")
&& processedRequest.getCatalogs().contains("catx")
What would you like to see in a unit test?
public String getDefaultCatalog() | ||
{ | ||
return defaultCatalog; | ||
} | ||
|
||
public Set<String> getCatalogs() | ||
{ | ||
return catalogs; | ||
} | ||
|
||
public Set<String> getSchemas() | ||
{ | ||
return schemas; | ||
} | ||
|
||
public Set<String> getCatalogSchemas() | ||
{ | ||
return catalogSchemas; | ||
} |
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.
Remove unused methods.
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.
These are intended for use in routing rules, but are unused in the Trino Gateway code. I added more cases to routing_rules_processed_request.yml
to cover getDefaultCatalog()
and getCatalogSchemas()
and provide usage examples.
gateway-ha/src/main/java/io/trino/gateway/ha/router/ProcessedRequest.java
Outdated
Show resolved
Hide resolved
b6fa7cf
to
c100ecd
Compare
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 have my first round of comments and questions in.
gateway-ha/src/main/java/io/trino/gateway/ha/router/ProcessedRequest.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/router/RuleReloadingRoutingGroupSelector.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/test/java/io/trino/gateway/ha/router/TestRoutingGroupSelector.java
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/router/ProcessedRequest.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/router/ProcessedRequest.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/ha/router/RuleReloadingRoutingGroupSelector.java
Outdated
Show resolved
Hide resolved
Are you handling the DDL statements like create and drop table? They are useful in case of |
ba2f218
to
73c5ae6
Compare
2627287
to
8f2e510
Compare
@vishalya DDL statements are handled, please check provideTableExtractionQueries for the full list of statement types. LMK if I missed any. |
Description
Extract information from the HttpServeletRequest into a class more convenient for usage in routing logic.
Additional context and related issues
Routing rules are authored in the MVEL language used by the jeasy rules engine. MVEL has limited functionality in comparison to standard Java, and cannot use libraries not included with the JDK. This PR passes an additional
processedRequest
object to the rules engine with getters for the user, query type, and SQL details such as referenced tables and schemas.Release notes
( ) This is not user-visible or is 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:
* Process request to facilitate rule writing