github-actions[bot] commented on code in PR #67899: URL: https://github.com/apache/doris/pull/67899#discussion_r4429901003
########## regression-test/suites/arrow_flight_sql_p0/test_table_types_over_arrow_flight.groovy: ########## @@ -0,0 +1,135 @@ +// 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. + +import org.apache.arrow.driver.jdbc.shaded.org.apache.arrow.flight.CallOption +import org.apache.arrow.driver.jdbc.shaded.org.apache.arrow.flight.CallOptions +import org.apache.arrow.driver.jdbc.shaded.org.apache.arrow.flight.FlightClient +import org.apache.arrow.driver.jdbc.shaded.org.apache.arrow.flight.Location +import org.apache.arrow.driver.jdbc.shaded.org.apache.arrow.flight.sql.FlightSqlClient +import org.apache.arrow.driver.jdbc.shaded.org.apache.arrow.memory.RootAllocator +import org.apache.arrow.driver.jdbc.shaded.org.apache.arrow.vector.types.pojo.ArrowType +import org.apache.doris.regression.util.JdbcUtils + +import java.util.concurrent.TimeUnit + +suite("test_table_types_over_arrow_flight", "arrow_flight_sql") { + def flightConn = context.getArrowFlightSqlConnection() + def runOnFlight = { String statement -> + JdbcUtils.executeToList(flightConn, statement) + } + runOnFlight "USE `${context.dbName}`" + runOnFlight "DROP VIEW IF EXISTS test_flight_table_types_view" + runOnFlight "DROP TABLE IF EXISTS test_flight_table_types_base" + runOnFlight """ + CREATE TABLE test_flight_table_types_base (id INT) + DISTRIBUTED BY HASH(id) BUCKETS 1 + PROPERTIES ("replication_num" = "1") + """ + runOnFlight """ + CREATE VIEW test_flight_table_types_view AS + SELECT id FROM test_flight_table_types_base + """ + + // These JDBC metadata methods consume Flight GetTableTypes/GetTables streams. + // SQL result snapshots alone do not exercise those metadata RPCs. + def metadata = flightConn.getMetaData() + def typeRows = metadata.getTableTypes() + try { + def types = [] + while (typeRows.next()) { + types.add(typeRows.getString("TABLE_TYPE")) + } + assertEquals(["BASE TABLE", "SYSTEM VIEW", "VIEW"], types) + } finally { + typeRows.close() + } + + def getTables = { List<String> types -> + def rows = metadata.getTables("internal", context.dbName, "test_flight_table_types%", + types == null ? null : types.toArray(new String[0])) + try { + def tables = [] + while (rows.next()) { + assertEquals("internal", rows.getString("TABLE_CAT")) + assertEquals(context.dbName, rows.getString("TABLE_SCHEM")) + tables.add([rows.getString("TABLE_NAME"), rows.getString("TABLE_TYPE")]) + } + return tables.sort { left, right -> left[0] <=> right[0] } + } finally { + rows.close() + } + } + + def baseTable = ["test_flight_table_types_base", "BASE TABLE"] + def view = ["test_flight_table_types_view", "VIEW"] + assertEquals([baseTable, view], getTables(null)) + assertEquals([baseTable, view], getTables([])) + assertEquals([baseTable], getTables(["BASE TABLE"])) + assertEquals([view], getTables(["VIEW"])) + // Both orders must work: forwarding only the first type loses or leaks rows. + assertEquals([baseTable, view], getTables(["VIEW", "BASE TABLE"])) + assertEquals([baseTable, view], getTables(["BASE TABLE", "VIEW"])) + assertEquals([], getTables(["SYSTEM VIEW"])) + assertEquals([], getTables(["UNKNOWN"])) + assertEquals([view], getTables(["UNKNOWN", "VIEW", "VIEW"])) + // Flight SQL calls TABLE, VIEW and SYSTEM TABLE the commonly supported names, and clients send them + // in either case: the ADBC Flight SQL driver asks for "table", JDBC tools for "TABLE". + assertEquals([baseTable], getTables(["TABLE"])) + assertEquals([baseTable], getTables(["table"])) + assertEquals([baseTable, view], getTables(["table", "view"])) + + // The same RPCs from a plain Flight SQL client, called repeatedly: GetFlightInfo carries the + // GetTableTypes schema and one endpoint, DoGet returns the same sorted types every time, and + // the session still answers a query afterwards. + def config = context.config.otherConfigs + def location = Location.forGrpcInsecure(config.get("extArrowFlightSqlHost"), + Integer.parseInt(config.get("extArrowFlightSqlPort"))) + new RootAllocator().withCloseable { allocator -> + FlightClient.builder(allocator, location).build().withCloseable { flight -> + def token = flight.authenticateBasicToken(config.get("extArrowFlightSqlUser"), Review Comment: [P2] Close the raw Flight SQL session before closing the client. authenticateBasicToken opens a Doris session in the shared connection pool, and closing the transport/allocator does not call FlightSqlProducer.closeSession; the session remains registered until CloseSession, KILL CONNECTION, or wait_timeout. This suite therefore leaks one connection slot per run and can exhaust the Flight/user quota or interfere with later suites. Keep the token and call client.closeSession(new CloseSessionRequest(), token) in a finally block before closing flight. -- 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]
