diff --git a/agents/drivers/oracle-10g/src/main/java/com/dbx/agent/oracle10g/Oracle10gAgent.java b/agents/drivers/oracle-10g/src/main/java/com/dbx/agent/oracle10g/Oracle10gAgent.java index 5c74825645..cb5494fc17 100644 --- a/agents/drivers/oracle-10g/src/main/java/com/dbx/agent/oracle10g/Oracle10gAgent.java +++ b/agents/drivers/oracle-10g/src/main/java/com/dbx/agent/oracle10g/Oracle10gAgent.java @@ -145,15 +145,9 @@ public final class Oracle10gAgent extends BaseDatabaseAgent { public List listTables(String schema) { return unchecked(() -> { List result = new ArrayList<>(); - String sql = "SELECT o.OBJECT_NAME," - + " CASE o.OBJECT_TYPE WHEN 'VIEW' THEN 'VIEW' ELSE 'TABLE' END AS TABLE_TYPE," - + " c.COMMENTS" - + " FROM ALL_OBJECTS o" - + " LEFT JOIN ALL_TAB_COMMENTS c ON c.OWNER = o.OWNER AND c.TABLE_NAME = o.OBJECT_NAME" - + " WHERE o.OWNER = ? AND o.OBJECT_TYPE IN ('TABLE','VIEW')" - + " ORDER BY o.OBJECT_NAME"; - try (PreparedStatement stmt = requireConnected().prepareStatement(sql)) { + try (PreparedStatement stmt = requireConnected().prepareStatement(listTablesSql())) { stmt.setString(1, schema); + stmt.setString(2, schema); try (ResultSet rs = stmt.executeQuery()) { while (rs.next()) { result.add(new TableInfo(rs.getString(1), rs.getString(2), rs.getString(3))); @@ -164,15 +158,32 @@ public final class Oracle10gAgent extends BaseDatabaseAgent { }); } + static String listTablesSql() { + return "SELECT OBJECT_NAME, TABLE_TYPE, COMMENTS" + + " FROM (" + + " SELECT t.TABLE_NAME AS OBJECT_NAME," + + " 'TABLE' AS TABLE_TYPE," + + " CAST(NULL AS VARCHAR2(4000)) AS COMMENTS" + + " FROM ALL_TABLES t" + + " WHERE t.OWNER = ?" + + " AND t.NESTED = 'NO'" + + " UNION ALL" + + " SELECT o.OBJECT_NAME," + + " 'VIEW' AS TABLE_TYPE," + + " CAST(NULL AS VARCHAR2(4000)) AS COMMENTS" + + " FROM ALL_OBJECTS o" + + " WHERE o.OWNER = ?" + + " AND o.OBJECT_TYPE = 'VIEW'" + + " ) ORDER BY OBJECT_NAME"; + } + @Override public List listObjects(String schema) { return unchecked(() -> { List result = new ArrayList<>(); - String sql = "SELECT OBJECT_NAME, OBJECT_TYPE FROM ALL_OBJECTS" - + " WHERE OWNER = ? AND OBJECT_TYPE IN ('TABLE', 'VIEW', 'PROCEDURE', 'FUNCTION')" - + " ORDER BY CASE OBJECT_TYPE WHEN 'TABLE' THEN 0 WHEN 'VIEW' THEN 1 WHEN 'PROCEDURE' THEN 2 ELSE 3 END, OBJECT_NAME"; - try (PreparedStatement stmt = requireConnected().prepareStatement(sql)) { + try (PreparedStatement stmt = requireConnected().prepareStatement(listObjectsSql())) { stmt.setString(1, schema); + stmt.setString(2, schema); try (ResultSet rs = stmt.executeQuery()) { while (rs.next()) { result.add(new ObjectInfo(rs.getString(1), rs.getString(2), schema, null)); @@ -183,6 +194,23 @@ public final class Oracle10gAgent extends BaseDatabaseAgent { }); } + static String listObjectsSql() { + return "SELECT OBJECT_NAME, OBJECT_TYPE" + + " FROM (" + + " SELECT t.TABLE_NAME AS OBJECT_NAME," + + " 'TABLE' AS OBJECT_TYPE" + + " FROM ALL_TABLES t" + + " WHERE t.OWNER = ?" + + " AND t.NESTED = 'NO'" + + " UNION ALL" + + " SELECT o.OBJECT_NAME," + + " o.OBJECT_TYPE" + + " FROM ALL_OBJECTS o" + + " WHERE o.OWNER = ?" + + " AND o.OBJECT_TYPE IN ('VIEW', 'PROCEDURE', 'FUNCTION')" + + " ) ORDER BY CASE OBJECT_TYPE WHEN 'TABLE' THEN 0 WHEN 'VIEW' THEN 1 WHEN 'PROCEDURE' THEN 2 ELSE 3 END, OBJECT_NAME"; + } + @Override public ObjectSource getObjectSource(String schema, String name, String objectType) { return unchecked(() -> { diff --git a/agents/drivers/oracle-10g/src/test/java/com/dbx/agent/oracle10g/Oracle10gAgentTest.java b/agents/drivers/oracle-10g/src/test/java/com/dbx/agent/oracle10g/Oracle10gAgentTest.java index b6d9cd3e78..1ae52ef144 100644 --- a/agents/drivers/oracle-10g/src/test/java/com/dbx/agent/oracle10g/Oracle10gAgentTest.java +++ b/agents/drivers/oracle-10g/src/test/java/com/dbx/agent/oracle10g/Oracle10gAgentTest.java @@ -15,6 +15,7 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.util.Arrays; import java.util.List; +import java.util.Locale; import java.util.function.Function; import java.util.stream.Collectors; import org.junit.jupiter.api.Assertions; @@ -89,6 +90,26 @@ class Oracle10gAgentTest extends JdbcFakeExecutionBehaviorTest { ); } + @Test + void listTablesSqlUsesSplitDictionaryQuery() { + String sql = Oracle10gAgent.listTablesSql().toUpperCase(Locale.ROOT); + + Assertions.assertTrue(sql.contains("ALL_TABLES"), sql); + Assertions.assertTrue(sql.contains("ALL_OBJECTS"), sql); + Assertions.assertTrue(sql.contains("UNION ALL"), sql); + Assertions.assertFalse(sql.contains("ALL_TAB_COMMENTS"), sql); + } + + @Test + void listObjectsSqlUsesSplitDictionaryQuery() { + String sql = Oracle10gAgent.listObjectsSql().toUpperCase(Locale.ROOT); + + Assertions.assertTrue(sql.contains("ALL_TABLES"), sql); + Assertions.assertTrue(sql.contains("ALL_OBJECTS"), sql); + Assertions.assertTrue(sql.contains("UNION ALL"), sql); + Assertions.assertFalse(sql.contains("ALL_TAB_COMMENTS"), sql); + } + @Test void loadsRoutineSourceFromDbmsMetadata() { Oracle10gAgent agent = new Oracle10gAgent(); diff --git a/agents/drivers/oracle-go/main.go b/agents/drivers/oracle-go/main.go index 2db1c4fc7e..4219ca9b86 100644 --- a/agents/drivers/oracle-go/main.go +++ b/agents/drivers/oracle-go/main.go @@ -42,6 +42,48 @@ ORDER BY CASE WHEN username = SYS_CONTEXT('USERENV', 'SESSION_USER') THEN 1 ELSE 2 END, username` +const oracleListTablesSQL = ` +SELECT OBJECT_NAME, TABLE_TYPE, COMMENTS +FROM ( +SELECT t.TABLE_NAME AS OBJECT_NAME, + 'TABLE' AS TABLE_TYPE, + CAST(NULL AS VARCHAR2(4000)) AS COMMENTS +FROM ALL_TABLES t +WHERE t.OWNER = :1 + AND t.NESTED = 'NO' +UNION ALL +SELECT o.OBJECT_NAME, + 'VIEW' AS TABLE_TYPE, + CAST(NULL AS VARCHAR2(4000)) AS COMMENTS +FROM ALL_OBJECTS o +WHERE o.OWNER = :2 + AND o.OBJECT_TYPE = 'VIEW' +) +ORDER BY OBJECT_NAME` +const oracleListObjectsSQL = ` +SELECT OBJECT_NAME, OBJECT_TYPE, COMMENTS +FROM ( +SELECT t.TABLE_NAME AS OBJECT_NAME, + 'TABLE' AS OBJECT_TYPE, + CAST(NULL AS VARCHAR2(4000)) AS COMMENTS +FROM ALL_TABLES t +WHERE t.OWNER = :1 + AND t.NESTED = 'NO' +UNION ALL +SELECT o.OBJECT_NAME, + o.OBJECT_TYPE, + CAST(NULL AS VARCHAR2(4000)) AS COMMENTS +FROM ALL_OBJECTS o +WHERE o.OWNER = :2 + AND o.OBJECT_TYPE IN ('VIEW', 'PROCEDURE', 'FUNCTION', 'PACKAGE') +) +ORDER BY CASE OBJECT_TYPE + WHEN 'TABLE' THEN 0 + WHEN 'VIEW' THEN 1 + WHEN 'PROCEDURE' THEN 2 + WHEN 'FUNCTION' THEN 3 + ELSE 4 +END, OBJECT_NAME` type request struct { ID json.RawMessage `json:"id"` @@ -565,15 +607,7 @@ func (s *server) listTables(schema string) ([]tableInfo, error) { if err != nil { return nil, err } - rows, err := s.queryRows(` -SELECT /*+ NO_QUERY_TRANSFORMATION */ o.OBJECT_NAME, - CASE o.OBJECT_TYPE WHEN 'VIEW' THEN 'VIEW' ELSE 'TABLE' END AS TABLE_TYPE, - c.COMMENTS -FROM ALL_OBJECTS o -LEFT JOIN ALL_TAB_COMMENTS c ON c.OWNER = o.OWNER AND c.TABLE_NAME = o.OBJECT_NAME -WHERE o.OWNER = :1 - AND o.OBJECT_TYPE IN ('TABLE', 'VIEW') -ORDER BY o.OBJECT_NAME`, []any{schema}) + rows, err := s.queryRows(oracleListTablesSQL, []any{schema, schema}) if err != nil { if isOraclePGALimitError(err) { return []tableInfo{}, nil @@ -597,13 +631,7 @@ func (s *server) listObjects(schema string) ([]objectInfo, error) { if err != nil { return nil, err } - rows, err := s.queryRows(` -SELECT /*+ NO_QUERY_TRANSFORMATION */ o.OBJECT_NAME, o.OBJECT_TYPE, c.COMMENTS -FROM ALL_OBJECTS o -LEFT JOIN ALL_TAB_COMMENTS c ON c.OWNER = o.OWNER AND c.TABLE_NAME = o.OBJECT_NAME -WHERE o.OWNER = :1 - AND o.OBJECT_TYPE IN ('TABLE', 'VIEW', 'PROCEDURE', 'FUNCTION', 'PACKAGE') -ORDER BY o.OBJECT_TYPE, o.OBJECT_NAME`, []any{schema}) + rows, err := s.queryRows(oracleListObjectsSQL, []any{schema, schema}) if err != nil { if isOraclePGALimitError(err) { return []objectInfo{}, nil diff --git a/agents/drivers/oracle-go/main_test.go b/agents/drivers/oracle-go/main_test.go index 3830f92963..04d07fd69c 100644 --- a/agents/drivers/oracle-go/main_test.go +++ b/agents/drivers/oracle-go/main_test.go @@ -265,6 +265,34 @@ func TestListDatabasesSQLUsesUserDictionaryInsteadOfObjectDictionary(t *testing. } } +func TestListTablesSQLUsesSplitDictionaryQuery(t *testing.T) { + sqlText := strings.ToUpper(oracleListTablesSQL) + + if !strings.Contains(sqlText, "ALL_TABLES") || !strings.Contains(sqlText, "ALL_OBJECTS") { + t.Fatalf("table listing should split tables and views, got: %s", oracleListTablesSQL) + } + if !strings.Contains(sqlText, "UNION ALL") { + t.Fatalf("table listing should union table and view metadata, got: %s", oracleListTablesSQL) + } + if strings.Contains(sqlText, "ALL_TAB_COMMENTS") { + t.Fatalf("table listing should not load comments during refresh, got: %s", oracleListTablesSQL) + } +} + +func TestListObjectsSQLUsesSplitDictionaryQuery(t *testing.T) { + sqlText := strings.ToUpper(oracleListObjectsSQL) + + if !strings.Contains(sqlText, "ALL_TABLES") || !strings.Contains(sqlText, "ALL_OBJECTS") { + t.Fatalf("object listing should split tables from other objects, got: %s", oracleListObjectsSQL) + } + if !strings.Contains(sqlText, "UNION ALL") { + t.Fatalf("object listing should union object metadata, got: %s", oracleListObjectsSQL) + } + if strings.Contains(sqlText, "ALL_TAB_COMMENTS") { + t.Fatalf("object listing should not load comments during refresh, got: %s", oracleListObjectsSQL) + } +} + func TestIsOraclePGALimitError(t *testing.T) { if !isOraclePGALimitError(errors.New("ORA-04036: PGA memory used by the instance exceeds PGA_AGGREGATE_LIMIT")) { t.Fatal("expected ORA-04036 to be detected") diff --git a/agents/drivers/oracle/src/main/java/com/dbx/agent/oracle/OracleAgent.java b/agents/drivers/oracle/src/main/java/com/dbx/agent/oracle/OracleAgent.java index c495dcbcdc..5305faa5db 100644 --- a/agents/drivers/oracle/src/main/java/com/dbx/agent/oracle/OracleAgent.java +++ b/agents/drivers/oracle/src/main/java/com/dbx/agent/oracle/OracleAgent.java @@ -216,19 +216,10 @@ public class OracleAgent extends BaseDatabaseAgent { public List listTables(String schema) { return unchecked(() -> { try { - String sql = """ - SELECT /*+ NO_QUERY_TRANSFORMATION */ o.OBJECT_NAME, - CASE o.OBJECT_TYPE WHEN 'VIEW' THEN 'VIEW' ELSE 'TABLE' END AS TABLE_TYPE, - c.COMMENTS - FROM ALL_OBJECTS o - LEFT JOIN ALL_TAB_COMMENTS c ON c.OWNER = o.OWNER AND c.TABLE_NAME = o.OBJECT_NAME - WHERE o.OWNER = ? AND o.OBJECT_TYPE IN ('TABLE','VIEW') - ORDER BY o.OBJECT_NAME - """.stripIndent().trim(); - List result = new ArrayList<>(); - try (var stmt = requireConnected().prepareStatement(sql)) { + try (var stmt = requireConnected().prepareStatement(listTablesSql())) { stmt.setString(1, schema); + stmt.setString(2, schema); try (ResultSet rs = stmt.executeQuery()) { while (rs.next()) { result.add(new TableInfo(rs.getString(1), rs.getString(2), rs.getString(3))); @@ -245,19 +236,36 @@ public class OracleAgent extends BaseDatabaseAgent { }); } + static String listTablesSql() { + return """ + SELECT OBJECT_NAME, TABLE_TYPE, COMMENTS + FROM ( + SELECT t.TABLE_NAME AS OBJECT_NAME, + 'TABLE' AS TABLE_TYPE, + CAST(NULL AS VARCHAR2(4000)) AS COMMENTS + FROM ALL_TABLES t + WHERE t.OWNER = ? + AND t.NESTED = 'NO' + UNION ALL + SELECT o.OBJECT_NAME, + 'VIEW' AS TABLE_TYPE, + CAST(NULL AS VARCHAR2(4000)) AS COMMENTS + FROM ALL_OBJECTS o + WHERE o.OWNER = ? + AND o.OBJECT_TYPE = 'VIEW' + ) + ORDER BY OBJECT_NAME + """.stripIndent().trim(); + } + @Override public List listObjects(String schema) { return unchecked(() -> { try { - String sql = """ - SELECT /*+ NO_QUERY_TRANSFORMATION */ OBJECT_NAME, OBJECT_TYPE FROM ALL_OBJECTS - WHERE OWNER = ? AND OBJECT_TYPE IN ('TABLE', 'VIEW', 'PROCEDURE', 'FUNCTION') - ORDER BY CASE OBJECT_TYPE WHEN 'TABLE' THEN 0 WHEN 'VIEW' THEN 1 WHEN 'PROCEDURE' THEN 2 ELSE 3 END, OBJECT_NAME - """.stripIndent().trim(); - List result = new ArrayList<>(); - try (var stmt = requireConnected().prepareStatement(sql)) { + try (var stmt = requireConnected().prepareStatement(listObjectsSql())) { stmt.setString(1, schema); + stmt.setString(2, schema); try (ResultSet rs = stmt.executeQuery()) { while (rs.next()) { result.add(new ObjectInfo(rs.getString(1), rs.getString(2), schema, null)); @@ -274,6 +282,26 @@ public class OracleAgent extends BaseDatabaseAgent { }); } + static String listObjectsSql() { + return """ + SELECT OBJECT_NAME, OBJECT_TYPE + FROM ( + SELECT t.TABLE_NAME AS OBJECT_NAME, + 'TABLE' AS OBJECT_TYPE + FROM ALL_TABLES t + WHERE t.OWNER = ? + AND t.NESTED = 'NO' + UNION ALL + SELECT o.OBJECT_NAME, + o.OBJECT_TYPE + FROM ALL_OBJECTS o + WHERE o.OWNER = ? + AND o.OBJECT_TYPE IN ('VIEW', 'PROCEDURE', 'FUNCTION') + ) + ORDER BY CASE OBJECT_TYPE WHEN 'TABLE' THEN 0 WHEN 'VIEW' THEN 1 WHEN 'PROCEDURE' THEN 2 ELSE 3 END, OBJECT_NAME + """.stripIndent().trim(); + } + @Override public ObjectSource getObjectSource(String schema, String name, String objectType) { return unchecked(() -> { diff --git a/agents/drivers/oracle/src/test/java/com/dbx/agent/oracle/OracleAgentTest.java b/agents/drivers/oracle/src/test/java/com/dbx/agent/oracle/OracleAgentTest.java index a612e65f8c..c4b82a849c 100644 --- a/agents/drivers/oracle/src/test/java/com/dbx/agent/oracle/OracleAgentTest.java +++ b/agents/drivers/oracle/src/test/java/com/dbx/agent/oracle/OracleAgentTest.java @@ -65,6 +65,26 @@ class OracleAgentTest extends JdbcFakeExecutionBehaviorTest { Assertions.assertFalse(sql.contains("ALL_VIEWS")); } + @Test + void listTablesSqlUsesSplitDictionaryQuery() { + String sql = OracleAgent.listTablesSql().toUpperCase(Locale.ROOT); + + Assertions.assertTrue(sql.contains("ALL_TABLES"), sql); + Assertions.assertTrue(sql.contains("ALL_OBJECTS"), sql); + Assertions.assertTrue(sql.contains("UNION ALL"), sql); + Assertions.assertFalse(sql.contains("ALL_TAB_COMMENTS"), sql); + } + + @Test + void listObjectsSqlUsesSplitDictionaryQuery() { + String sql = OracleAgent.listObjectsSql().toUpperCase(Locale.ROOT); + + Assertions.assertTrue(sql.contains("ALL_TABLES"), sql); + Assertions.assertTrue(sql.contains("ALL_OBJECTS"), sql); + Assertions.assertTrue(sql.contains("UNION ALL"), sql); + Assertions.assertFalse(sql.contains("ALL_TAB_COMMENTS"), sql); + } + @Test void detectsPgaLimitError() { Assertions.assertTrue(OracleAgent.isPgaLimitError(new SQLException( diff --git a/crates/dbx-core/src/schema.rs b/crates/dbx-core/src/schema.rs index 5c31a161bc..0411d821b6 100644 --- a/crates/dbx-core/src/schema.rs +++ b/crates/dbx-core/src/schema.rs @@ -696,6 +696,36 @@ pub async fn get_table_comment_core( { let connections = state.connections.read().await; try_sqlserver!(connections, &pool_key, get_table_comment, schema, table); + if let Some(client) = extract_pool!(&connections, &pool_key, Agent) { + if db_config.as_ref().is_some_and(|config| { + matches!(config.db_type, DatabaseType::Oracle | DatabaseType::OceanbaseOracle) + }) { + let sql = oracle_table_comment_sql(schema, table); + let timeout = agent_metadata_timeout(db_config.as_ref()); + drop(connections); + let mut client = client.lock().await; + let result = client + .execute_query_with_timeout::( + agent_execute_query_params( + &sql, + Some(database), + Some(schema), + QueryExecutionOptions { + max_rows: Some(1), + fetch_size: None, + page_size: None, + result_session_id: None, + client_session_id: None, + timeout_secs: None, + execution_id: None, + }, + ), + timeout, + ) + .await?; + return oracle_table_comment_from_query_result(result); + } + } } let connections = state.connections.read().await; @@ -718,6 +748,22 @@ pub async fn get_table_comment_core( .await } +fn oracle_table_comment_sql(schema: &str, table: &str) -> String { + format!( + "SELECT COMMENTS FROM ALL_TAB_COMMENTS WHERE OWNER = {} AND TABLE_NAME = {} AND TABLE_TYPE IN ('TABLE', 'VIEW')", + sql_string(schema), + sql_string(table), + ) +} + +fn oracle_table_comment_from_query_result(result: db::QueryResult) -> Result, String> { + Ok(result + .rows + .first() + .and_then(|row| row.iter().find_map(|value| value.as_str().map(str::to_string))) + .filter(|value| !value.trim().is_empty())) +} + async fn list_tables_once( state: &AppState, connection_id: &str, @@ -1096,7 +1142,8 @@ mod tests { use super::{ clickhouse_metadata_database, deduplicate_column_infos, filter_mysql_system_databases_for_config, filter_table_infos, is_agent_postgres_metadata_fallback_config, normalize_information_schema_table_type, - presto_like_information_schema_tables_sql, presto_like_tables_from_query_result, + oracle_table_comment_from_query_result, oracle_table_comment_sql, presto_like_information_schema_tables_sql, + presto_like_tables_from_query_result, }; #[cfg(feature = "duckdb-bundled")] use super::{ @@ -1457,6 +1504,48 @@ mod tests { config.query_timeout_secs = 0; assert_eq!(super::agent_metadata_timeout(Some(&config)), None); } + + #[test] + fn oracle_table_comment_sql_targets_single_table_and_escapes_literals() { + let sql = oracle_table_comment_sql("APP'S", "USER'S"); + + assert!(sql.contains("ALL_TAB_COMMENTS")); + assert!(sql.contains("OWNER = 'APP''S'")); + assert!(sql.contains("TABLE_NAME = 'USER''S'")); + assert!(sql.contains("TABLE_TYPE IN ('TABLE', 'VIEW')")); + assert!(!sql.contains("ALL_OBJECTS")); + } + + #[test] + fn oracle_table_comment_from_query_result_returns_optional_non_blank_comment() { + let result = db::QueryResult { + columns: vec!["COMMENTS".to_string()], + column_types: Vec::new(), + column_sortables: Vec::new(), + rows: vec![vec![serde_json::json!("Customer table")]], + affected_rows: 0, + execution_time_ms: 0, + truncated: false, + session_id: None, + has_more: false, + }; + + assert_eq!(oracle_table_comment_from_query_result(result).unwrap().as_deref(), Some("Customer table")); + + let empty = db::QueryResult { + columns: vec!["COMMENTS".to_string()], + column_types: Vec::new(), + column_sortables: Vec::new(), + rows: vec![vec![serde_json::json!(" ")]], + affected_rows: 0, + execution_time_ms: 0, + truncated: false, + session_id: None, + has_more: false, + }; + + assert_eq!(oracle_table_comment_from_query_result(empty).unwrap(), None); + } } pub async fn list_objects_core(