Skip to content

[Enhancement] Validate materialized view subqueries against SQL grammar deny list#5485

Merged
RyanL1997 merged 1 commit into
opensearch-project:mainfrom
RyanL1997:flint-mv-subquery-validation
May 29, 2026
Merged

[Enhancement] Validate materialized view subqueries against SQL grammar deny list#5485
RyanL1997 merged 1 commit into
opensearch-project:mainfrom
RyanL1997:flint-mv-subquery-validation

Conversation

@RyanL1997

@RyanL1997 RyanL1997 commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Description

Implement validateFlintExtensionQuery() to extract and validate the inner SQL subquery from CREATE MATERIALIZED VIEW statements against the grammar element deny list. Previously, this method was a no-op, meaning MV subqueries were not checked.

Also adds tumble and hop to the DATE_TIMESTAMP function type in FunctionType.java to align with Spark's classification of time windowing functions, ensuring these legitimate functions are not incorrectly blocked.

Changes

  • SQLQueryValidator.java — Implement validateFlintExtensionQuery() to extract the MV inner query via SQLQueryUtils.extractIndexDetails() and validate it through the existing validate() method.
  • FunctionType.java — Add tumble and hop to the DATE_TIMESTAMP function type (consistent with window and session_window already there).
  • SQLQueryValidatorTest.java — Add tests for MV subquery validation (blocked constructs rejected, legitimate queries pass including window/tumble/hop functions).
  • SparkQueryDispatcherTest.java — Add end-to-end dispatch tests verifying MV queries with window functions are dispatched successfully and blocked constructs are rejected.

Issues Resolved

N/A

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff.
  • All tests pass locally (./gradlew :async-query-core:test).

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

@RyanL1997
Validate Flint extension query subqueries against grammar deny list
024238d 
Implement validateFlintExtensionQuery() to extract and validate the
inner SQL subquery from CREATE MATERIALIZED VIEW statements. Previously,
this method was a no-op, meaning MV subqueries were not checked against
the grammar element deny list.

Also adds tumble and hop to the DATE_TIMESTAMP function type to align
with Spark's classification of time windowing functions.

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
@github-actions

github-actions Bot commented May 28, 2026

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. The code assumes extractIndexDetails always returns a non-null object, but this is not guaranteed by the visible code or documented in the method signature.

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

@github-actions

github-actions Bot commented May 28, 2026

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 is a critical bug that could cause runtime failures.

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 SQLQueryUtils.extractIndexDetails, we cannot definitively confirm this is a critical bug. The score reflects that this is a valid defensive programming practice that improves robustness, though the actual risk depends on the behavior of the called method.

Medium

@RyanL1997 RyanL1997 added enhancement New feature or request spark integration labels May 28, 2026
@RyanL1997 RyanL1997 changed the title Validate Flint extension query subqueries against grammar deny list [Enhancement] Validate Flint extension query subqueries against grammar deny list May 28, 2026
@RyanL1997 RyanL1997 changed the title [Enhancement] Validate Flint extension query subqueries against grammar deny list [Enhancement] Validate materialized view subqueries against SQL grammar deny list May 28, 2026
@RyanL1997 RyanL1997 marked this pull request as ready for review May 28, 2026 22:41
@RyanL1997 RyanL1997 merged commit 25529e7 into opensearch-project:main May 29, 2026
49 of 50 checks passed
@RyanL1997 RyanL1997 deleted the flint-mv-subquery-validation branch May 29, 2026 16:50
This was referenced May 29, 2026
Merged
Merged
RyanL1997 added a commit that referenced this pull request May 29, 2026
@RyanL1997
[Backport 3.7] Validate materialized view subqueries against SQL gram…
0d2e7c3 
…mar deny list (#5485) (#5489)
RyanL1997 added a commit that referenced this pull request Jun 2, 2026
@RyanL1997
[Backport 2.19] Validate materialized view subqueries against SQL gra…
9da0962 
…mmar deny list (#5485) (#5490)

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dai-chen dai-chen dai-chen approved these changes

@mengweieric mengweieric mengweieric approved these changes

@ps48 ps48 Awaiting requested review from ps48 ps48 is a code owner

@joshuali925 joshuali925 Awaiting requested review from joshuali925 joshuali925 is a code owner

@vamsimanohar vamsimanohar Awaiting requested review from vamsimanohar vamsimanohar is a code owner

@Swiddis Swiddis Awaiting requested review from Swiddis Swiddis is a code owner

@penghuo penghuo Awaiting requested review from penghuo penghuo is a code owner

@anirudha anirudha Awaiting requested review from anirudha anirudha is a code owner

@acarbonetto acarbonetto Awaiting requested review from acarbonetto acarbonetto is a code owner

@ykmr1224 ykmr1224 Awaiting requested review from ykmr1224 ykmr1224 is a code owner

@LantaoJin LantaoJin Awaiting requested review from LantaoJin LantaoJin is a code owner

@noCharger noCharger Awaiting requested review from noCharger noCharger is a code owner

@qianheng-aws qianheng-aws Awaiting requested review from qianheng-aws qianheng-aws is a code owner

@yuancu yuancu Awaiting requested review from yuancu yuancu is a code owner

@ahkcs ahkcs Awaiting requested review from ahkcs ahkcs is a code owner

@songkant-aws songkant-aws Awaiting requested review from songkant-aws songkant-aws is a code owner

Assignees

No one assigned

Labels

backport 2.19 backport 3.7 enhancement New feature or request spark integration

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RyanL1997 @dai-chen @mengweieric