Skip to content

[Backport 2.19-dev] Validate materialized view subqueries against SQL grammar deny list (#5485)#5616

Merged
Swiddis merged 1 commit into
opensearch-project:2.19-devfrom
RyanL1997:flint-validation-2.19-dev
Jul 10, 2026
Merged

[Backport 2.19-dev] Validate materialized view subqueries against SQL grammar deny list (#5485)#5616
Swiddis merged 1 commit into
opensearch-project:2.19-devfrom
RyanL1997:flint-validation-2.19-dev

Conversation

@RyanL1997

Copy link
Copy Markdown
Collaborator

Description

Validate materialized view subqueries against SQL grammar deny list.

Backport of #5490 (which backported #5485) to 2.19-dev.

Implements validateFlintExtensionQuery() (previously a no-op) to extract the inner SQL subquery from CREATE MATERIALIZED VIEW statements via SQLQueryUtils.extractIndexDetails() and validate it against the same grammar-element deny list used for standard SQL queries. Also adds tumble and hop to the DATE_TIMESTAMP function type in FunctionType.java (consistent with window/session_window) so legitimate time-windowing functions are not incorrectly blocked.

Related Issues

Check List

  • New functionality includes testing.
  • Commits are signed per the DCO using --signoff or -s.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…mmar deny list (opensearch-project#5485) (opensearch-project#5490)

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
(cherry picked from commit 9da0962)
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Null Pointer Risk

If SQLQueryUtils.extractIndexDetails(sqlQuery) returns null, calling indexQueryDetails.getMvQuery() will throw a NullPointerException. This can occur if the input query is malformed or not recognized by the extraction logic.

IndexQueryDetails indexQueryDetails = SQLQueryUtils.extractIndexDetails(sqlQuery);
String mvQuery = indexQueryDetails.getMvQuery();

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add null check for indexQueryDetails

Add null check for indexQueryDetails before accessing getMvQuery(). If
extractIndexDetails returns null, calling getMvQuery() will throw a
NullPointerException. This could happen with malformed or unexpected query formats.

async-query-core/src/main/java/org/opensearch/sql/spark/validator/SQLQueryValidator.java [49-55]

 public void validateFlintExtensionQuery(String sqlQuery, DataSourceType dataSourceType) {
   IndexQueryDetails indexQueryDetails = SQLQueryUtils.extractIndexDetails(sqlQuery);
-  String mvQuery = indexQueryDetails.getMvQuery();
-  if (mvQuery != null) {
-    validate(mvQuery, dataSourceType);
+  if (indexQueryDetails != null) {
+    String mvQuery = indexQueryDetails.getMvQuery();
+    if (mvQuery != null) {
+      validate(mvQuery, dataSourceType);
+    }
   }
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential NullPointerException if extractIndexDetails returns null. However, without seeing the implementation of extractIndexDetails, we cannot definitively confirm this is a critical issue. The suggestion is valid defensive programming practice, though it may be addressing a theoretical rather than actual risk depending on the method's contract.

Medium

@RyanL1997 RyanL1997 enabled auto-merge (squash) July 10, 2026 20:55
@Swiddis Swiddis disabled auto-merge July 10, 2026 21:02
@Swiddis Swiddis merged commit 69da3dc into opensearch-project:2.19-dev Jul 10, 2026
6 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants