Copilot commented on code in PR #16480: URL: https://github.com/apache/pinot/pull/16480#discussion_r2249125060
########## pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/TableNameExtractor.java: ########## @@ -0,0 +1,297 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.client; + +import java.util.HashSet; +import java.util.Set; +import javax.annotation.Nullable; +import org.apache.calcite.sql.SqlBasicCall; +import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlJoin; +import org.apache.calcite.sql.SqlKind; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlNodeList; +import org.apache.calcite.sql.SqlOrderBy; +import org.apache.calcite.sql.SqlSelect; +import org.apache.calcite.sql.SqlWith; +import org.apache.calcite.sql.SqlWithItem; +import org.apache.pinot.sql.parsers.CalciteSqlParser; +import org.apache.pinot.sql.parsers.SqlNodeAndOptions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +/** + * Helper class to extract table names from Calcite SqlNode tree. Review Comment: The class is missing a comprehensive class-level Javadoc that explains its purpose, main methods, and usage examples. Consider adding documentation that describes the AST traversal strategy and key functionality. ```suggestion * Utility class for extracting table names from SQL queries using Calcite's SQL AST. * <p> * This class provides methods to parse a SQL query and traverse its Abstract Syntax Tree (AST) * to identify all table names referenced in the query, including those in subqueries and joins. * It also handles Common Table Expressions (CTEs) by tracking their definitions and ensuring * that references to CTEs are not mistaken for physical table names. * </p> * * <h2>Main Methods</h2> * <ul> * <li>{@link #resolveTableName(String)}: Parses a SQL query string and returns an array of all table names used in the query.</li> * <li>{@link #extractTableNames(SqlNode)}: Traverses a Calcite {@code SqlNode} tree to collect table names.</li> * <li>{@link #getTableNames()}: Returns the set of table names found during traversal.</li> * </ul> * * <h2>AST Traversal Strategy</h2> * <ul> * <li>The class recursively traverses the Calcite {@code SqlNode} tree, visiting nodes such as {@code SqlSelect}, {@code SqlJoin}, and {@code SqlWith}.</li> * <li>When a table identifier is found in a FROM clause, it is added to the set of table names, unless it matches a known CTE name.</li> * <li>CTE definitions are tracked to distinguish between CTE references and actual table names.</li> * <li>Subqueries and nested queries are handled by recursive traversal.</li> * </ul> * * <h2>Usage Example</h2> * <pre>{@code * String query = "WITH cte AS (SELECT * FROM foo) SELECT * FROM cte JOIN bar ON cte.id = bar.id"; * String[] tableNames = TableNameExtractor.resolveTableName(query); * // tableNames will contain ["foo", "bar"] * }</pre> ``` ########## pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/TableNameExtractorTest.java: ########## @@ -0,0 +1,613 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.client; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertTrue; + + +/** + * Tests for the TableNameExtractor class. + */ +public class TableNameExtractorTest { + + @Test + public void testResolveTableNameWithSingleQuery() { + // Test that single queries work correctly + String singleQuery = "SELECT * FROM myTable WHERE id > 100"; + + String[] tableNames = TableNameExtractor.resolveTableName(singleQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 1, "Should resolve exactly one table"); + assertEquals(tableNames[0], "myTable", "Should resolve the correct table name"); + } + + @Test + public void testResolveTableNameWithSingleStatementAlias() { + String singleStatementQuery = "SELECT stats.* FROM airlineStats stats LIMIT 10"; + String[] tableNames = TableNameExtractor.resolveTableName(singleStatementQuery); + + assertNotNull(tableNames); + assertEquals(tableNames.length, 1); + assertEquals(tableNames[0], "airlineStats"); + } + + @Test + public void testResolveTableNameWithMultiStatementQuery() { + // Test the fix for issue #11823: CalciteSQLParser error with multi-statement queries + String multiStatementQuery = "SET useMultistageEngine=true;\nSELECT stats.* FROM airlineStats stats LIMIT 10"; + + // This should not throw a ClassCastException anymore + String[] tableNames = TableNameExtractor.resolveTableName(multiStatementQuery); + + // Should successfully resolve the table name + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 1, "Should resolve exactly one table"); + assertEquals(tableNames[0], "airlineStats", "Should resolve the correct table name"); + } + + @Test + public void testResolveTableNameWithMultipleSetStatements() { + // Test with multiple SET statements + String multiSetQuery = "SET useMultistageEngine=true;\nSET timeoutMs=10000;\nSELECT * FROM testTable"; + + String[] tableNames = TableNameExtractor.resolveTableName(multiSetQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 1, "Should resolve exactly one table"); + assertEquals(tableNames[0], "testTable", "Should resolve the correct table name"); + } + + @Test + public void testResolveTableNameWithMultipleSetStatementsAndJoin() { + String multiStatementQuery = "SET useMultistageEngine=true;\nSET maxRowsInJoin=1000;\n" + + "SELECT stats.* FROM airlineStats stats LIMIT 10"; + String[] tableNames = TableNameExtractor.resolveTableName(multiStatementQuery); + + assertNotNull(tableNames, "Table names should be resolved for queries with multiple SET statements"); + assertEquals(tableNames.length, 1); + assertEquals(tableNames[0], "airlineStats"); + } + + @Test + public void testResolveTableNameWithJoin() { + // Test with JOIN queries + String joinQuery = "SELECT * FROM table1 t1 JOIN table2 t2 ON t1.id = t2.id"; + + String[] tableNames = TableNameExtractor.resolveTableName(joinQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 2, "Should resolve two tables"); + assertTrue(Arrays.asList(tableNames).contains("table1"), "Should contain table1"); + assertTrue(Arrays.asList(tableNames).contains("table2"), "Should contain table2"); + } + + @Test + public void testResolveTableNameWithJoinQueryAndSetStatements() { + String joinQuery = "SET useMultistageEngine=true;\n" + + "SELECT a.col1, b.col2 FROM tableA a JOIN tableB b ON a.id = b.id"; + String[] tableNames = TableNameExtractor.resolveTableName(joinQuery); + + assertNotNull(tableNames, "Table names should be resolved for join queries with SET statements"); + assertEquals(tableNames.length, 2); + + Set<String> expectedTableNames = new HashSet<>(Arrays.asList("tableA", "tableB")); + Set<String> actualTableNames = new HashSet<>(Arrays.asList(tableNames)); + assertEquals(actualTableNames, expectedTableNames); + } + + @Test + public void testResolveTableNameWithExplicitAlias() { + // Test with explicit AS alias + String aliasQuery = "SELECT u.name FROM users AS u WHERE u.active = true"; + + String[] tableNames = TableNameExtractor.resolveTableName(aliasQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 1, "Should resolve exactly one table"); + assertEquals(tableNames[0], "users", "Should resolve the actual table name, not the alias"); + } + + @Test + public void testResolveTableNameWithImplicitAlias() { + // Test with implicit alias (no AS keyword) + String implicitAliasQuery = "SELECT o.id, u.name FROM orders o JOIN users u ON o.user_id = u.id"; + + String[] tableNames = TableNameExtractor.resolveTableName(implicitAliasQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 2, "Should resolve two tables"); + assertTrue(Arrays.asList(tableNames).contains("orders"), "Should contain orders table"); + assertTrue(Arrays.asList(tableNames).contains("users"), "Should contain users table"); + } + + @Test + public void testResolveTableNameWithCTE() { + // Test with Common Table Expression (CTE) + String cteQuery = "WITH active_users AS (SELECT * FROM users WHERE active = true) " + + "SELECT au.name FROM active_users au JOIN orders o ON au.id = o.user_id"; + + String[] tableNames = TableNameExtractor.resolveTableName(cteQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 2, "Should resolve two tables"); + assertTrue(Arrays.asList(tableNames).contains("users"), "Should contain users table from CTE"); + assertTrue(Arrays.asList(tableNames).contains("orders"), "Should contain orders table"); + } + + @Test + public void testResolveTableNameWithNestedCTE() { + // Test with nested CTEs + String nestedCteQuery = "WITH user_orders AS (" + + " SELECT u.id, u.name, o.order_date " + + " FROM users u JOIN orders o ON u.id = o.user_id" + + "), recent_orders AS (" + + " SELECT * FROM user_orders WHERE order_date > '2023-01-01'" + + ") " + + "SELECT ro.name FROM recent_orders ro JOIN products p ON ro.id = p.user_id"; + + String[] tableNames = TableNameExtractor.resolveTableName(nestedCteQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 3, "Should resolve three tables"); + assertTrue(Arrays.asList(tableNames).contains("users"), "Should contain users table"); + assertTrue(Arrays.asList(tableNames).contains("orders"), "Should contain orders table"); + assertTrue(Arrays.asList(tableNames).contains("products"), "Should contain products table"); + } + + @Test + public void testResolveTableNameWithSubqueryAlias() { + // Test with subquery alias + String subqueryQuery = "SELECT t.name FROM (SELECT * FROM users WHERE active = true) AS t " + + "JOIN orders o ON t.id = o.user_id"; + + String[] tableNames = TableNameExtractor.resolveTableName(subqueryQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 2, "Should resolve two tables"); + assertTrue(Arrays.asList(tableNames).contains("users"), "Should contain users table from subquery"); + assertTrue(Arrays.asList(tableNames).contains("orders"), "Should contain orders table"); + } + + @Test + public void testResolveTableNameWithComplexJoinAndAliases() { + // Test with multiple JOINs and various alias styles + String complexQuery = "SELECT u.name, o.total, p.title " + + "FROM users AS u " + + "INNER JOIN orders o ON u.id = o.user_id " + + "LEFT JOIN order_items oi ON o.id = oi.order_id " + + "RIGHT JOIN products AS p ON oi.product_id = p.id " + + "WHERE u.active = true"; + + String[] tableNames = TableNameExtractor.resolveTableName(complexQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 4, "Should resolve four tables"); + assertTrue(Arrays.asList(tableNames).contains("users"), "Should contain users table"); + assertTrue(Arrays.asList(tableNames).contains("orders"), "Should contain orders table"); + assertTrue(Arrays.asList(tableNames).contains("order_items"), "Should contain order_items table"); + assertTrue(Arrays.asList(tableNames).contains("products"), "Should contain products table"); + } + + @Test + public void testResolveTableNameWithJoinConditionSubquery() { + // Test with subquery in join condition + String joinSubqueryQuery = "SELECT u.name, o.total " + + "FROM users u " + + "JOIN orders o ON u.id = o.user_id " + + "AND o.id IN (SELECT order_id FROM order_items WHERE quantity > 5)"; + + String[] tableNames = TableNameExtractor.resolveTableName(joinSubqueryQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 3, "Should resolve three tables"); + assertTrue(Arrays.asList(tableNames).contains("users"), "Should contain users table"); + assertTrue(Arrays.asList(tableNames).contains("orders"), "Should contain orders table"); + assertTrue(Arrays.asList(tableNames).contains("order_items"), + "Should contain order_items table from subquery"); + } + + @Test + public void testResolveTableNameWithOrderBy() { + // Test with ORDER BY clause + String orderByQuery = "SELECT * FROM users ORDER BY name"; + String[] tableNames = TableNameExtractor.resolveTableName(orderByQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 1, "Should resolve exactly one table"); + assertEquals(tableNames[0], "users", "Should resolve the correct table name"); + } + + @Test + public void testResolveTableNameWithOrderBySubquery() { + // Test with subquery in ORDER BY clause (rare but possible) + String orderBySubqueryQuery = "SELECT * FROM users u ORDER BY " + + "(SELECT COUNT(*) FROM orders o WHERE o.user_id = u.id)"; + String[] tableNames = TableNameExtractor.resolveTableName(orderBySubqueryQuery); + + assertNotNull(tableNames, "Table names should not be null"); + assertEquals(tableNames.length, 2, "Should resolve two tables"); + assertTrue(Arrays.asList(tableNames).contains("users"), "Should contain users table"); + assertTrue(Arrays.asList(tableNames).contains("orders"), "Should contain orders table"); + } + + @Test + public void testResolveTableNameWithInvalidQuery() { + String invalidQuery = "INVALID SQL QUERY"; + String[] tableNames = TableNameExtractor.resolveTableName(invalidQuery); + + // Should return null when query cannot be parsed (fallback to default broker selector) + assertNull(tableNames); + } + + @Test + public void testResolveTableNameWithOnlySetStatements() { + String onlySetQuery = "SET useMultistageEngine=true;"; + String[] tableNames = TableNameExtractor.resolveTableName(onlySetQuery); + + // Should return null when there's no actual query statement + assertNull(tableNames); + } + + @Test + public void testResolveTableNameWithNullQuery() { + String[] tableNames = TableNameExtractor.resolveTableName(null); + + // Should return null when query is null + assertNull(tableNames); + } + + @Test + public void testResolveTableNameWithEmptyQuery() { + String[] tableNames = TableNameExtractor.resolveTableName(""); + + // Should return null when query is empty + assertNull(tableNames); + } + + /** + * Data provider for SQL queries and their expected table names. + * This makes it easy to add new test cases by simply adding entries to this array. + * + * @return Object array containing: [testName, sqlQuery, expectedTableNames] Review Comment: [nitpick] The data provider method documentation could be improved to explain the test data structure format more clearly, including what each array element represents. ```suggestion * Each entry in the returned array is an Object[] of length 3, structured as follows: * <ul> * <li><b>testName</b> (String): A descriptive name for the test case.</li> * <li><b>sqlQuery</b> (String): The SQL query to be tested.</li> * <li><b>expectedTableNames</b> (String[]): The expected table names to be extracted from the query, * or {@code null} if no table names are expected (e.g., for invalid or empty queries).</li> * </ul> * This makes it easy to add new test cases by simply adding entries to this array. * * @return Object[][] where each Object[] contains: [testName (String), sqlQuery (String), expectedTableNames (String[] or null)] ``` ########## pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/TableNameExtractor.java: ########## @@ -0,0 +1,297 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.client; + +import java.util.HashSet; +import java.util.Set; +import javax.annotation.Nullable; +import org.apache.calcite.sql.SqlBasicCall; +import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlJoin; +import org.apache.calcite.sql.SqlKind; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlNodeList; +import org.apache.calcite.sql.SqlOrderBy; +import org.apache.calcite.sql.SqlSelect; +import org.apache.calcite.sql.SqlWith; +import org.apache.calcite.sql.SqlWithItem; +import org.apache.pinot.sql.parsers.CalciteSqlParser; +import org.apache.pinot.sql.parsers.SqlNodeAndOptions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +/** + * Helper class to extract table names from Calcite SqlNode tree. + */ +public class TableNameExtractor { + private static final Logger LOGGER = LoggerFactory.getLogger(TableNameExtractor.class); + + /** + * Returns the name of all the tables used in a sql query. + * + * @param query The SQL query string to analyze + * @return name of all the tables used in a sql query, or null if parsing fails + */ + @Nullable + public static String[] resolveTableName(String query) { + SqlNodeAndOptions sqlNodeAndOptions; + try { + sqlNodeAndOptions = CalciteSqlParser.compileToSqlNodeAndOptions(query); + } catch (Exception e) { + LOGGER.error("Cannot parse table name from query: {}. Fallback to broker selector default.", query, e); + return null; + } + try { + Set<String> tableNames = extractTableNamesFromMultiStageQuery(sqlNodeAndOptions.getSqlNode()); + if (tableNames != null) { + return tableNames.toArray(new String[0]); + } + } catch (Exception e) { + LOGGER.error("Cannot extract table name from query: {}. Fallback to broker selector default.", query, e); + } + return null; + } + + /** + * Extracts table names from a multi-stage query using Calcite SQL AST traversal. + * + * @param sqlNode The root SqlNode of the parsed query + * @return Set of table names found in the query + */ + private static Set<String> extractTableNamesFromMultiStageQuery(SqlNode sqlNode) { + TableNameExtractor extractor = new TableNameExtractor(); + try { + extractor.extractTableNames(sqlNode); + return extractor.getTableNames(); + } catch (Exception e) { + LOGGER.error("Failed to extract table names from multi-stage query", e); + return null; + } + } + + private final Set<String> _tableNames = new HashSet<>(); + private final Set<String> _cteNames = new HashSet<>(); + private boolean _inFromClause = false; + + public Set<String> getTableNames() { + return _tableNames; + } + Review Comment: This public method lacks Javadoc documentation. It should explain the recursive AST traversal approach, the node types it handles, and how the context flags affect extraction. ```suggestion /** * Recursively traverses the given Calcite SQL AST node to extract table names. * <p> * This method dispatches on the type of the provided {@link SqlNode}, handling the following node types: * <ul> * <li>{@link SqlWith} - Handles common table expressions (CTEs).</li> * <li>{@link SqlOrderBy} - Handles ORDER BY clauses.</li> * <li>{@link SqlWithItem} - Handles individual CTE definitions.</li> * <li>{@link SqlSelect} - Handles SELECT statements, including FROM, WHERE, etc.</li> * <li>{@link SqlJoin} - Handles JOIN clauses.</li> * <li>{@link SqlBasicCall} - Handles function calls and expressions.</li> * <li>{@link SqlIdentifier} - Handles identifiers, which may be table names.</li> * <li>{@link SqlNodeList} - Handles lists of nodes, such as select lists or group by clauses.</li> * </ul> * The extraction process is context-sensitive. For example, the {@code _inFromClause} flag is used to determine * whether an identifier should be interpreted as a table name (when traversing FROM clauses) or ignored (when * traversing other parts of the query). CTE names are tracked to avoid treating them as table names. * * @param node The root {@link SqlNode} to traverse for table name extraction. */ ``` ########## pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/TableNameExtractor.java: ########## @@ -0,0 +1,297 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.client; + +import java.util.HashSet; +import java.util.Set; +import javax.annotation.Nullable; +import org.apache.calcite.sql.SqlBasicCall; +import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlJoin; +import org.apache.calcite.sql.SqlKind; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlNodeList; +import org.apache.calcite.sql.SqlOrderBy; +import org.apache.calcite.sql.SqlSelect; +import org.apache.calcite.sql.SqlWith; +import org.apache.calcite.sql.SqlWithItem; +import org.apache.pinot.sql.parsers.CalciteSqlParser; +import org.apache.pinot.sql.parsers.SqlNodeAndOptions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +/** + * Helper class to extract table names from Calcite SqlNode tree. + */ +public class TableNameExtractor { + private static final Logger LOGGER = LoggerFactory.getLogger(TableNameExtractor.class); + + /** + * Returns the name of all the tables used in a sql query. + * + * @param query The SQL query string to analyze + * @return name of all the tables used in a sql query, or null if parsing fails + */ + @Nullable + public static String[] resolveTableName(String query) { + SqlNodeAndOptions sqlNodeAndOptions; + try { + sqlNodeAndOptions = CalciteSqlParser.compileToSqlNodeAndOptions(query); + } catch (Exception e) { + LOGGER.error("Cannot parse table name from query: {}. Fallback to broker selector default.", query, e); + return null; + } + try { + Set<String> tableNames = extractTableNamesFromMultiStageQuery(sqlNodeAndOptions.getSqlNode()); + if (tableNames != null) { + return tableNames.toArray(new String[0]); + } + } catch (Exception e) { + LOGGER.error("Cannot extract table name from query: {}. Fallback to broker selector default.", query, e); + } + return null; + } + + /** + * Extracts table names from a multi-stage query using Calcite SQL AST traversal. + * + * @param sqlNode The root SqlNode of the parsed query + * @return Set of table names found in the query + */ + private static Set<String> extractTableNamesFromMultiStageQuery(SqlNode sqlNode) { + TableNameExtractor extractor = new TableNameExtractor(); + try { + extractor.extractTableNames(sqlNode); + return extractor.getTableNames(); + } catch (Exception e) { + LOGGER.error("Failed to extract table names from multi-stage query", e); + return null; + } + } + + private final Set<String> _tableNames = new HashSet<>(); + private final Set<String> _cteNames = new HashSet<>(); + private boolean _inFromClause = false; + Review Comment: This public method should have Javadoc documentation explaining what it returns and when it should be called. ```suggestion /** * Returns the set of table names extracted from the SQL node tree. * <p> * This method should be called after {@link #extractTableNames(SqlNode)} has been invoked * to populate the set of table names. * * @return Set of table names found in the SQL node tree */ ``` -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
