From 80f53ba5ec32921e64bba63bbe2fc83f1840ec1e Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 31 Jan 2025 13:50:50 -0500 Subject: [PATCH 1/6] Implement contains in AREL to fix param binding Active record wants '?' as a bind placeholder. Without this it will not provide bind variables to exec. Oracle requires :N style bind parameters, so it a translation is required. Something went wrong in the translation between rails 7.1 and rails 7.2 so that the where("CONTAINS(table.col, ?, 0) >0", term) was no longer getting it's bind parameter translated to ':a1' like it should be. This change drops down to AREL nodes which ensures that the contains query is getting passed through the visitor pattern, specifically hitting the visit_Arel_Nodes_BindParam method to convert the param to ':a1'. I still do not understand what changed in rails, but this workaround is effective. --- .../oracle_enhanced/context_index.rb | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/active_record/connection_adapters/oracle_enhanced/context_index.rb b/lib/active_record/connection_adapters/oracle_enhanced/context_index.rb index ab04cc1f5..513d12974 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/context_index.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/context_index.rb @@ -330,8 +330,23 @@ module ContextIndexClassMethods # Add context index condition. def contains(column, query, options = {}) score_label = options[:label].to_i || 1 - where("CONTAINS(#{connection.quote_table_name(column)}, ?, #{score_label}) > 0", query). - order(Arel.sql("SCORE(#{score_label}) DESC")) + quoted_column = connection.quote_table_name(column) + + # Create an Arel node for the CONTAINS function + contains_node = Arel::Nodes::NamedFunction.new( + "CONTAINS", + [ + Arel::Nodes::SqlLiteral.new(quoted_column), + Arel::Nodes::BindParam.new(query), + Arel::Nodes::SqlLiteral.new(score_label.to_s) + ] + ) + + # Create comparison node: CONTAINS(...) > 0 + condition = Arel::Nodes::GreaterThan.new(contains_node, Arel::Nodes::SqlLiteral.new("0")) + + # Create the where clause and order by score + where(condition).order(Arel.sql("SCORE(#{score_label}) DESC")) end end end From 9f9687fa4c92fc6305eb457f3f8a51de1039a64b Mon Sep 17 00:00:00 2001 From: Andy Date: Wed, 19 Feb 2025 19:19:43 -0500 Subject: [PATCH 2/6] Use the new preprocess_query method First test failure involves a missing method `transform_query` Seems to have been removed in this rails commit: fd24e5bfc9540fc00764a59ddf39a993bbd63ba2 https://github.com/rails/rails/commit/fd24e5bfc9540fc00764a59ddf39a993bbd63ba2 This method was replaced with `preprocess_query`. --- .../oracle_enhanced/database_statements.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb index 81af330e0..932cf379b 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb @@ -10,13 +10,13 @@ module DatabaseStatements # Executes a SQL statement def execute(sql, name = nil, async: false, allow_retry: false) - sql = transform_query(sql) + sql = preprocess_query(sql) log(sql, name, async: async) { _connection.exec(sql, allow_retry: allow_retry) } end def exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false) - sql = transform_query(sql) + sql = preprocess_query(sql) type_casted_binds = type_casted_binds(binds) From 9f2c1fed9687d62fbe28a8c19e51f7b2aa99025a Mon Sep 17 00:00:00 2001 From: Andy Date: Wed, 19 Feb 2025 19:20:22 -0500 Subject: [PATCH 3/6] Use the instrumenter method. @instrumenter is not defined. It was moved to a method. https://github.com/rails/rails/commit/dc522a3e6933e014c691e06c5d866d6351d6640d --- .../connection_adapters/oracle_enhanced/dbms_output.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_record/connection_adapters/oracle_enhanced/dbms_output.rb b/lib/active_record/connection_adapters/oracle_enhanced/dbms_output.rb index 69420186b..fe7056e0d 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/dbms_output.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/dbms_output.rb @@ -32,7 +32,7 @@ def dbms_output_enabled? private def log(sql, name = "SQL", binds = [], type_casted_binds = [], statement_name = nil, async: false, &block) - @instrumenter.instrument( + instrumenter.instrument( "sql.active_record", sql: sql, name: name, From baf9fd488b18197621b9ee047c11251c89f6abe6 Mon Sep 17 00:00:00 2001 From: Andy Date: Sun, 23 Feb 2025 21:28:42 -0500 Subject: [PATCH 4/6] Rails 8 support. - Add `write_query?` implementation (mimicking postgres) to support the ability to prevent writes to a database - https://github.com/rails/rails/commit/f39d72d5267baed1000932831cda98503d1e1047 - Replace the local implementation of execute, exec_query and its alias internal_exec_query with the new interface defined here https://github.com/rails/rails/commit/fd24e5bfc9540fc00764a59ddf39a993bbd63ba2#diff-e6fd03cad5e3437c76b6c5db106359124dc9feab8341ade39b9ae54af001fac9 of `raw_execute`, `cast_result`, and `affected_rows`. To support the affected_rows count this also had to add a `row_count` method to the coi and jdbc cursors. - without_prepared_statement? was removed https://github.com/rails/rails/commit/2306c10e7a9397f8096e49def97ed47125a4499f --- .../oracle_enhanced/database_statements.rb | 100 ++++++++++++------ .../oracle_enhanced/jdbc_connection.rb | 4 + .../oracle_enhanced/oci_connection.rb | 4 + 3 files changed, 73 insertions(+), 35 deletions(-) diff --git a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb index 932cf379b..005f03532 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb @@ -8,58 +8,79 @@ module DatabaseStatements # # see: abstract/database_statements.rb - # Executes a SQL statement - def execute(sql, name = nil, async: false, allow_retry: false) - sql = preprocess_query(sql) + READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp( + :close, :declare, :fetch, :move, :set, :show + ) # :nodoc: + private_constant :READ_QUERY + + def write_query?(sql) # :nodoc: + !READ_QUERY.match?(sql) + rescue ArgumentError # Invalid encoding + !READ_QUERY.match?(sql.b) + end - log(sql, name, async: async) { _connection.exec(sql, allow_retry: allow_retry) } + # Executes a SQL statement + def execute(...) + super end - def exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false) + # Low level execution of a SQL statement on the connection returning adapter specific result object. + def raw_execute(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: false) sql = preprocess_query(sql) type_casted_binds = type_casted_binds(binds) + with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn| + log(sql, name, binds, type_casted_binds, async: async) do + cursor = nil + cached = false + with_retry do + if binds.nil? || binds.empty? + cursor = conn.prepare(sql) + else + unless @statements.key? sql + @statements[sql] = conn.prepare(sql) + end - log(sql, name, binds, type_casted_binds, async: async) do - cursor = nil - cached = false - with_retry do - if without_prepared_statement?(binds) - cursor = _connection.prepare(sql) - else - unless @statements.key? sql - @statements[sql] = _connection.prepare(sql) - end - - cursor = @statements[sql] - - cursor.bind_params(type_casted_binds) + cursor = @statements[sql] + cursor.bind_params(type_casted_binds) - cached = true + cached = true + end + cursor.exec end - cursor.exec - end - - if (name == "EXPLAIN") && sql.start_with?("EXPLAIN") - res = true - else columns = cursor.get_col_names.map do |col_name| oracle_downcase(col_name) end + rows = [] - fetch_options = { get_lob_value: (name != "Writable Large Object") } - while row = cursor.fetch(fetch_options) - rows << row + if sql =~ /\A\s*SELECT/i # This seems a naive way to detect queries that will have row results. + fetch_options = { get_lob_value: (name != "Writable Large Object") } + while row = cursor.fetch(fetch_options) + rows << row + end end - res = build_result(columns: columns, rows: rows) + + affected_rows_count = cursor.row_count + + cursor.close unless cached + + { columns: columns, rows: rows, affected_rows_count: affected_rows_count } end + end + end - cursor.close unless cached - res + def cast_result(result) + if result.nil? + ActiveRecord::Result.empty + else + ActiveRecord::Result.new(result[:columns], result[:rows]) end end - alias_method :internal_exec_query, :exec_query + + def affected_rows(result) + result[:affected_rows_count] + end def supports_explain? true @@ -106,7 +127,7 @@ def exec_insert(sql, name = nil, binds = [], pk = nil, sequence_name = nil, retu cursor = nil returning_id_col = returning_id_index = nil with_retry do - if without_prepared_statement?(binds) + if binds.nil? || binds.empty? cursor = _connection.prepare(sql) else unless @statements.key?(sql) @@ -146,7 +167,7 @@ def exec_update(sql, name = nil, binds = []) log(sql, name, binds, type_casted_binds) do with_retry do cached = false - if without_prepared_statement?(binds) + if binds.nil? || binds.empty? cursor = _connection.prepare(sql) else if @statements.key?(sql) @@ -289,6 +310,15 @@ def with_retry raise end end + + def handle_warnings(sql) + @notice_receiver_sql_warnings.each do |warning| + next if warning_ignored?(warning) + + warning.sql = sql + ActiveRecord.db_warnings_action.call(warning) + end + end end end end diff --git a/lib/active_record/connection_adapters/oracle_enhanced/jdbc_connection.rb b/lib/active_record/connection_adapters/oracle_enhanced/jdbc_connection.rb index 39ed78e65..998a4b867 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/jdbc_connection.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/jdbc_connection.rb @@ -385,6 +385,10 @@ def column_names end alias :get_col_names :column_names + def row_count + @raw_statement.getUpdateCount + end + def fetch(options = {}) if @raw_result_set.next get_lob_value = options[:get_lob_value] diff --git a/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb b/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb index 329536ae6..e0801c3a8 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb @@ -158,6 +158,10 @@ def get_col_names @raw_cursor.get_col_names end + def row_count + @raw_cursor.row_count + end + def fetch(options = {}) if row = @raw_cursor.fetch get_lob_value = options[:get_lob_value] From 98df39deb413c6de56ed74f2dcfd0bf20e10acd2 Mon Sep 17 00:00:00 2001 From: Andy Date: Sun, 23 Feb 2025 21:35:35 -0500 Subject: [PATCH 5/6] Change ruby requirements for rails 8. drop 3.1 add 3.4 --- .github/workflows/test.yml | 3 +-- .github/workflows/test_11g.yml | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9c2e29073..53d777c89 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,7 +13,7 @@ jobs: strategy: fail-fast: false matrix: - # Rails 7.2 requires Ruby 3.1 or higeher. + # Rails 8.0 requires Ruby 3.2 or higeher. # CI pending the following matrix until JRuby 9.4 that supports Ruby 2.7 will be released. # https://github.com/jruby/jruby/issues/6464 # - jruby, @@ -22,7 +22,6 @@ jobs: '3.4', '3.3', '3.2', - '3.1', ] env: ORACLE_HOME: /opt/oracle/instantclient_23_8 diff --git a/.github/workflows/test_11g.yml b/.github/workflows/test_11g.yml index dbc1cd4b1..d9f52cea0 100644 --- a/.github/workflows/test_11g.yml +++ b/.github/workflows/test_11g.yml @@ -13,7 +13,7 @@ jobs: strategy: fail-fast: false matrix: - # Rails 7.2 requires Ruby 3.1 or higeher. + # Rails 8.0 requires Ruby 3.2 or higeher. # CI pending the following matrix until JRuby 9.4 that supports Ruby 2.7 will be released. # https://github.com/jruby/jruby/issues/6464 # - jruby, @@ -22,7 +22,6 @@ jobs: '3.4', '3.3', '3.2', - '3.1', ] env: ORACLE_HOME: /opt/oracle/instantclient_21_15 From 562712a60c554f4f5e4c73239cb1ee3a40cb0e63 Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 23 May 2025 12:35:56 -0400 Subject: [PATCH 6/6] Improve result row detection. Use OCI8's cursor.type to tell us if it is a select statement. https://www.rubydoc.info/gems/ruby-oci8/OCI8/Cursor#type-instance_method --- .../oracle_enhanced/database_statements.rb | 2 +- .../connection_adapters/oracle_enhanced/oci_connection.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb index 005f03532..805ade843 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb @@ -54,7 +54,7 @@ def raw_execute(sql, name = "SQL", binds = [], prepare: false, async: false, all end rows = [] - if sql =~ /\A\s*SELECT/i # This seems a naive way to detect queries that will have row results. + if cursor.select_statement? fetch_options = { get_lob_value: (name != "Writable Large Object") } while row = cursor.fetch(fetch_options) rows << row diff --git a/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb b/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb index e0801c3a8..ae454953f 100644 --- a/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb +++ b/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb @@ -162,6 +162,10 @@ def row_count @raw_cursor.row_count end + def select_statement? + @raw_cursor.type == :select_stmt + end + def fetch(options = {}) if row = @raw_cursor.fetch get_lob_value = options[:get_lob_value]