rishi-rana commented on PR #39882: URL: https://github.com/apache/shardingsphere/pull/39882#issuecomment-6228332632
Thanks, both reviewed. 1. `StandardDatabaseProxyConnector.java:313-317` — fixed in 7a14edf78d4. The real bug was one level deeper than the explicit-owner fix: each table's schema is now read from its own binder-resolved `TableBoundInfo.getOriginalSchema()`, not from the statement-level `SchemaRefreshUtils.getActualSchemaName(sqlStatementContext)`, which just takes whichever schema among all tables in the statement was bound first. For `DROP TABLE s1.a, b`, that previously leaked `a`'s bound schema onto `b`. Also fixed the `Optional#map` parameter naming. 2. `SET SCHEMA` — this is a pre-existing defect, not something this PR introduces. `PostgreSQLDDLStatementVisitor.visitAlterDefinitionClause` has no branch for the bare `SET SCHEMA name` alternative in the grammar (only `alterTableActions`, `renameColumnSpecification`, `renameTableSpecification`), so it parses successfully but is silently dropped from the AST — `AlterTableStatement.getRenameTable()` stays empty. I confirmed this already breaks the existing synchronous `AlterTablePushDownMetaDataRefresher` identically today, independent of any transaction: it reloads the old schema/table location (now empty after the physical move) and overwrites it with a fallback empty shell, never creating the destination's metadata. Fixing the root cause needs a new AST segment plus visitor and binder support in the `parser` module, which this PR doesn't touch. I don't think it's right to patch that silently inside this PR's existing scope. Two ways to handle it, your call: - File it separately against the existing synchronous path (it's not specific to transactions), and this PR's documented contract notes SET SCHEMA as an existing, unrelated limitation rather than something this PR newly breaks. - Or, if you'd rather this PR explicitly excludes schema-move ALTER TABLE from the permitted transactional set until the parser gap is fixed — I don't currently have a way to detect that case from the parsed statement to exclude it, since the parser drops the information before it ever reaches this code. 3. Still awaiting your sign-off on the support contract. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
