Repository navigation
[Query] Release UDF query scope when IoTDBLocal rejects a non-query statement - #19240
Merged
Merged
Conversation
IoTDBLocal.query() runs its statement through Coordinator.execution() with readOnlyInternalQuery set. The QueryExecution is constructed and analyzed first, then InternalQueryExecutor.validateReadOnlyQuery() rejects a statement that is not a query. At that point the execution has not been registered in queryExecutionMap and has not been started, so nothing ever called stopAndCleanup() on it. The UDF query scope opened for its analysis was never finalized, and the class loaders pinned for the UDFs used by the statement stayed open after CREATE FUNCTION deprecated them. Clean up the rejected execution before rethrowing the same exception, so the error returned to the caller does not change. Add a regression test that drives Coordinator.execution() with a statement whose analysis marks it as a write.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19240 +/- ##
=========================================
Coverage 48.94% 48.94%
Complexity 689 689
=========================================
Files 5589 5589
Lines 413857 413861 +4
Branches 54594 54594
=========================================
+ Hits 202544 202581 +37
+ Misses 211313 211280 -33 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.




Description
Follow-up to #19107. When
IoTDBLocal.query()rejects a statement that is not a query, theQueryExecutionbuilt for that statement never finalizes its UDF query scope. The class loaders pinned for the UDFs used by the statement therefore stay open afterCREATE FUNCTIONdeprecates them.Scenario
A table scalar UDF
localqcallsIoTDBLocal.query()inbeforeStartwith a write statement that uses the external UDFlazy_f:The statement is rejected as expected with
Only query is supported for IoTDBLocal query interface. AfterSELECT localq(v) FROM oneruns 5 times, aCREATE FUNCTIONdeprecates the class loader that definedlazy_f:lazy_factiveQueriesCount=5, deprecated, never closedactiveQueriesCount=0, deprecated, closedRead-only internal queries such as
SELECT lazy_f(v) FROM db.talready leftactiveQueriesCount=0, whether the UDF closes the result set or leaves it to the framework. They are unchanged.Cause
Coordinator.execution()constructs theQueryExecutionfirst. Its analysis runs inside the execution's UDF query scope, wherereflect()pins the class loader that defined each external UDF. Only then doesInternalQueryExecutor.validateReadOnlyQuery()throwSemanticExceptionfor a statement that is not a query. At that point the execution is not inqueryExecutionMapand has not been started, socleanupQueryExecution()cannot find it and nothing callsstopAndCleanup(). The scope is never finalized.Fix
Coordinator.execution()now callsstopAndCleanup()on the rejected execution and rethrows the same exception, so the message and status code returned to the caller do not change.QueryExecutionthat was never started,stopAndCleanup()releases what the analysis acquired and finalizes the UDF query scope.ConfigExecution.stopAndCleanup()does nothing.queryExecutionMap.put()/start()can throw. Construction failures are already handled byanalyzeWithCleanupOnFailure(), andstart()finalizes the scope itself when it fails.Validation
The new
CoordinatorTest#testRejectedReadOnlyInternalQueryClosesUDFQueryScopecallsCoordinator.execution(..., readOnlyInternalQuery = true, ...)with a realQueryExecutionwhose analysis marks the statement asWRITE. It checks that:SemanticExceptionis thrown;On master (54bff3d) it fails at the scope check, and with the fix it passes.
With the fix on master, these 7 classes pass (52 tests) with Spotless and Checkstyle enabled:
CoordinatorTest,QueryExecutionRetryTest,QueryOwnershipTest,LoadTsFileAnalyzerDiagnosticsTest,LoadTsFileDataTypeConverterSchemaTest,CteMaterializerTestandCteSubqueryTest.The scenario above ran on a 1C1D cluster built from 449a2b6, the head merged as Protect UDF classloader lifetime with query scopes #19107, once without and once with the fix. With the fix on that head,
IoTDBLocalIT(TableSimpleIT) passed: 22 tests passed, includingtestRejectMutatingStatementsViaIoTDBLocal, and 1 was skipped because it requires cluster execution. The query execution, IoTDBLocal and UDF class loader code is unchanged on master since that head.This PR has:
for an unfamiliar reader.
for code coverage.
Key changed/added classes (or packages if there are too many classes) in this PR
CoordinatorCoordinatorTest(new)🤖 Generated with Claude Code