diff --git a/CHANGELOG.md b/CHANGELOG.md index 655318d..c240648 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to LowQuery are documented here. The format follows [Keep a ## [Unreleased] +### Fixed +- Oracle: running a query that returns a CLOB or BLOB again in the same session could fail with ORA-01002 (fetch out of sequence) ([#17](https://github.com/LowBloat/LowQuery/issues/17)). It happened when the server parsed the query again, which on Oracle 23ai often came by the third run of a query such as `dbms_metadata.get_ddl`, and on every run after one that had failed, until reconnecting. Both causes were in the Oracle driver LowQuery ships and are fixed there; Show DDL no longer needs to bypass the statement cache. + ## [0.5.1] - 2026-10-22 ### Security diff --git a/src-tauri/src/drivers/integration_tests.rs b/src-tauri/src/drivers/integration_tests.rs index efb4457..d1d7462 100644 --- a/src-tauri/src/drivers/integration_tests.rs +++ b/src-tauri/src/drivers/integration_tests.rs @@ -415,6 +415,142 @@ async fn oracle_wide_rows_with_repeated_values() { } } +/// Queries returning CLOBs run again and again in one session, through the +/// driver's statement cache. LOBs are fetched as text, which takes a define +/// round trip on the first run. ORA-01002 (fetch out of sequence) followed +/// when that run failed, and when the server parsed and described the query +/// again on a later run. +#[tokio::test] +#[ignore] +async fn oracle_clob_queries_run_repeatedly() { + let Some((profile, pw)) = ora_profile() else { + return; + }; + let s = Session::open(&profile, pw.as_deref()) + .await + .expect("connection"); + let schema = profile.user.to_uppercase(); + let _ = s.execute(&req("drop table lq_clob purge", 0)).await; + s.execute(&req( + "create table lq_clob (id number primary key, doc clob)", + 0, + )) + .await + .unwrap(); + // Values longer than 4000 characters, which no VARCHAR2 define can hold. + s.execute(&req( + "insert into lq_clob + select level, to_clob(rpad('a', 4000, 'a')) || rpad('b', 4000, 'b') || level + from dual connect by level <= 3", + 0, + )) + .await + .unwrap(); + s.execute(&req("commit", 0)).await.unwrap(); + + let long_doc = |id: usize| format!("{}{}{id}", "a".repeat(4000), "b".repeat(4000)); + type Check = Box>])>; + let queries: [(&str, Check); 4] = [ + ( + "select dbms_metadata.get_ddl('TABLE', 'LQ_CLOB') from dual", + Box::new(|rows| { + assert_eq!(rows.len(), 1); + let ddl = rows[0][0].as_deref().unwrap_or_default(); + assert!( + ddl.contains("CREATE TABLE") && ddl.contains("LQ_CLOB"), + "{ddl}" + ); + }), + ), + ( + "select to_clob('LowQuery ' || level) from dual connect by level <= 3", + Box::new(|rows| { + let got: Vec<_> = rows.iter().map(|r| r[0].clone()).collect(); + let want: Vec<_> = (1..=3).map(|i| Some(format!("LowQuery {i}"))).collect(); + assert_eq!(got, want); + }), + ), + ( + "select id, doc from lq_clob order by id", + Box::new(move |rows| { + assert_eq!(rows.len(), 3); + for (i, row) in rows.iter().enumerate() { + assert_eq!(row[0].as_deref(), Some((i + 1).to_string().as_str())); + assert_eq!(row[1].as_deref(), Some(long_doc(i + 1).as_str())); + } + }), + ), + // The first row only: the cursor is left open with rows to fetch. + ( + "select doc from lq_clob order by id", + Box::new(move |rows| { + assert_eq!(rows.len(), 1); + assert_eq!(rows[0][0].as_deref(), Some(long_doc(1).as_str())); + }), + ), + ]; + // New statistics invalidate the cursors on LQ_CLOB, as statistics + // feedback does on its own: the next run parses the query again and the + // server describes its columns again, as CLOB. + let invalidate = + "begin dbms_stats.gather_table_stats(user, 'LQ_CLOB', no_invalidate => false); end;"; + for (n, (sql, check)) in queries.iter().enumerate() { + let limit = if n == 3 { 1 } else { 100 }; + for run in 1..=5 { + if run == 3 { + s.execute(&req(invalidate, 0)).await.unwrap(); + } + let r = s + .execute(&req(sql, limit)) + .await + .unwrap_or_else(|e| panic!("{sql}: run {run}: {e}")); + check(&r.rows); + } + } + // A query that fails while its LOB column is defined (ORA-01476 here), + // then succeeds once the data changes. + let _ = s.execute(&req("drop table lq_clob_flag purge", 0)).await; + s.execute(&req("create table lq_clob_flag (n number)", 0)) + .await + .unwrap(); + s.execute(&req("insert into lq_clob_flag values (0)", 0)) + .await + .unwrap(); + s.execute(&req("commit", 0)).await.unwrap(); + let flagged = "select to_clob('n=' || (select 1 / n from lq_clob_flag)) from dual"; + for run in 1..=2 { + let err = s.execute(&req(flagged, 100)).await.unwrap_err(); + assert!(err.to_string().contains("ORA-01476"), "run {run}: {err}"); + } + s.execute(&req("update lq_clob_flag set n = 1", 0)) + .await + .unwrap(); + s.execute(&req("commit", 0)).await.unwrap(); + for _ in 3..=5 { + assert_eq!(one_row(&s, flagged).await[0].as_deref(), Some("n=1")); + } + s.execute(&req("drop table lq_clob_flag purge", 0)) + .await + .unwrap(); + // Show DDL reads DBMS_METADATA with binds, five times in a row; the + // table has no comments, so one of its queries fails every time. + for run in 1..=5 { + let ddl = s + .object_ddl(&schema, "LQ_CLOB", "table") + .await + .unwrap_or_else(|e| panic!("Show DDL run {run}: {e}")); + assert!(ddl.contains("LQ_CLOB"), "{ddl}"); + } + // The session is still usable afterwards. + assert_eq!( + one_row(&s, "select count(*) from lq_clob").await[0].as_deref(), + Some("3") + ); + s.execute(&req("drop table lq_clob purge", 0)) + .await + .unwrap(); +} + #[tokio::test] #[ignore] async fn oracle_connects_through_tns_alias() { diff --git a/src-tauri/src/drivers/oracle.rs b/src-tauri/src/drivers/oracle.rs index 8656439..c9fcfc2 100644 --- a/src-tauri/src/drivers/oracle.rs +++ b/src-tauri/src/drivers/oracle.rs @@ -580,9 +580,7 @@ impl OraSession { } } -/// First column of every row, as text. The statement stays out of the -/// driver's cache: on 23ai, running a cached DBMS_METADATA query a third time -/// in a session fails with ORA-01002 (fetch out of sequence). +/// First column of every row, as text. fn first_texts(conn: &Connection, sql: &str, params: &[String]) -> AppResult> { let binds: Vec<&dyn oracledb::ToDbValue> = params .iter() @@ -591,7 +589,6 @@ fn first_texts(conn: &Connection, sql: &str, params: &[String]) -> AppResult + { + self.db_type = DbType::from_ora_type_and_csfrm( + constants::ORA_TYPE_NUM_LONG, + previous.db_type.csfrm, + ); + self.max_size = 0; + } + constants::ORA_TYPE_NUM_BLOB + if matches!( + previous_type, + constants::ORA_TYPE_NUM_RAW + | constants::ORA_TYPE_NUM_LONG_RAW + ) => + { + self.db_type = crate::DB_TYPE_LONG_RAW; + self.max_size = 0; + } + _ => {} + } + } + /// Returns a new structure suitable for defining CLOB/BLOB as /// string/bytes. pub(crate) fn define_metadata(&self) -> Metadata { diff --git a/src-tauri/vendor/oracledb/src/statement/cache.rs b/src-tauri/vendor/oracledb/src/statement/cache.rs index 2f5daf3..13bc3a3 100644 --- a/src-tauri/vendor/oracledb/src/statement/cache.rs +++ b/src-tauri/vendor/oracledb/src/statement/cache.rs @@ -131,6 +131,13 @@ impl StatementCache { !self.empty_slots.is_empty() || !self.free_used_slots.is_empty() } + /// Forgets the database cursor of a statement and closes it on the next + /// round trip (LowQuery patch). + pub(crate) fn clear_cursor(&mut self, info: &mut CachedStatement) { + self.close_cursor(info); + info.clear_cursor(); + } + /// Returns whether the statement cache has cursors that need to be closed. pub(crate) fn has_cursors_to_close(&self) -> bool { !self.cursors_to_close.is_empty() diff --git a/src-tauri/vendor/oracledb/src/statement/mod.rs b/src-tauri/vendor/oracledb/src/statement/mod.rs index 05eded8..35b4972 100644 --- a/src-tauri/vendor/oracledb/src/statement/mod.rs +++ b/src-tauri/vendor/oracledb/src/statement/mod.rs @@ -374,9 +374,17 @@ impl CachedStatement { if num_columns > 0 { resp.read_u8()?; } - self.out_metadata.clear(); - for _ in 0..num_columns { + // LowQuery patch: on a cursor that already ran, the columns described + // before tell whether it has a define for its LOB columns. + let mut previous = std::mem::take(&mut self.out_metadata); + if !self.has_cursor() { + previous.clear(); + } + for i in 0..num_columns { let mut metadata = Metadata::from_response(resp, client)?; + if let Some(previous) = previous.get(i as usize) { + metadata.adjust_for_previous(previous); + } if metadata.requires_define() { self.requires_define = true; self.no_prefetch = true;