From 29c8d6cc8a6e4e898690d5407043c64c00d616b6 Mon Sep 17 00:00:00 2001 From: Brian Thoman Date: Wed, 16 Oct 2024 11:35:36 -0400 Subject: [PATCH 1/7] Use IS [NOT] NULL instead of DBMS_LOB.COMPARE for nil CLOB/BLOB queries --- lib/arel/visitors/oracle_common.rb | 2 ++ .../oracle_enhanced/type/binary_spec.rb | 21 +++++++++++++++++++ .../oracle_enhanced/type/text_spec.rb | 21 +++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/lib/arel/visitors/oracle_common.rb b/lib/arel/visitors/oracle_common.rb index 594ff3a3c..504e01232 100644 --- a/lib/arel/visitors/oracle_common.rb +++ b/lib/arel/visitors/oracle_common.rb @@ -9,7 +9,9 @@ module OracleCommon # Fixes ORA-00932: inconsistent datatypes: expected - got CLOB def visit_Arel_Nodes_Equality(o, collector) left = o.left + return super unless %i(text binary).include?(cached_column_for(left)&.type) + return super if o.right.nil? # https://docs.oracle.com/cd/B19306_01/appdev.102/b14258/d_lob.htm#i1016668 # returns 0 when the comparison succeeds diff --git a/spec/active_record/oracle_enhanced/type/binary_spec.rb b/spec/active_record/oracle_enhanced/type/binary_spec.rb index 3f1cae8a3..dc98f4385 100644 --- a/spec/active_record/oracle_enhanced/type/binary_spec.rb +++ b/spec/active_record/oracle_enhanced/type/binary_spec.rb @@ -116,4 +116,25 @@ class ::TestEmployee < ActiveRecord::Base @employee.reload expect(@employee.binary_data).to eq(@binary_data) end + + it "should find serialized NULL BLOB data when queried with nil" do + TestEmployee.delete_all + TestEmployee.create!(binary_data: nil) + TestEmployee.create!(binary_data: @binary_data) + expect(TestEmployee.where(binary_data: nil)).to have_attributes(count: 1) + end + + it "should find serialized NULL BLOB data when queried with nil" do + TestSerializedHashEmployee.delete_all + TestSerializedHashEmployee.create!(binary_data: nil) + TestSerializedHashEmployee.create!(binary_data: { data: 'some data' }) + expect(TestSerializedHashEmployee.where(binary_data: nil)).to have_attributes(count: 1) + end + + it "should find serialized NULL BLOB data when queried with {}" do + TestSerializedHashEmployee.delete_all + TestSerializedHashEmployee.create!(binary_data: nil) + TestSerializedHashEmployee.create!(binary_data: { data: 'some data' }) + expect(TestSerializedHashEmployee.where(binary_data: {})).to have_attributes(count: 1) + end end diff --git a/spec/active_record/oracle_enhanced/type/text_spec.rb b/spec/active_record/oracle_enhanced/type/text_spec.rb index ccffd5e49..837576c7a 100644 --- a/spec/active_record/oracle_enhanced/type/text_spec.rb +++ b/spec/active_record/oracle_enhanced/type/text_spec.rb @@ -241,4 +241,25 @@ class ::TestSerializeEmployee < ActiveRecord::Base ) expect(Test2Employee.where(comments: search_data)).to have_attributes(count: 1) end + + it "should find NULL CLOB data when queried with nil" do + TestEmployee.delete_all + TestEmployee.create!(comments: nil) + TestEmployee.create!(comments: @char_data) + expect(TestEmployee.where(comments: nil)).to have_attributes(count: 1) + end + + it "should find serialized NULL CLOB data when queried with nil" do + Test2Employee.delete_all + Test2Employee.create!(comments: nil) + Test2Employee.create!(comments: { some: 'text' }) + expect(Test2Employee.where(comments: nil)).to have_attributes(count: 1) + end + + it "should find serialized NULL CLOB data when queried with {}" do + TestSerializedHashEmployee.delete_all + TestSerializedHashEmployee.create!(comments: nil) + TestSerializedHashEmployee.create!(comments: { some: 'text' }) + expect(TestSerializedHashEmployee.where(comments: {})).to have_attributes(count: 1) + end end From e871d2a40a89b94c38344dfb3a330432ac240cc2 Mon Sep 17 00:00:00 2001 From: Brian Thoman Date: Mon, 21 Oct 2024 15:57:46 -0400 Subject: [PATCH 2/7] Define test models for Serialized LOB columns --- .../active_record/oracle_enhanced/type/binary_spec.rb | 11 +++++++++++ spec/active_record/oracle_enhanced/type/text_spec.rb | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/spec/active_record/oracle_enhanced/type/binary_spec.rb b/spec/active_record/oracle_enhanced/type/binary_spec.rb index dc98f4385..b4335dd7b 100644 --- a/spec/active_record/oracle_enhanced/type/binary_spec.rb +++ b/spec/active_record/oracle_enhanced/type/binary_spec.rb @@ -15,6 +15,10 @@ end class ::TestEmployee < ActiveRecord::Base end + class ::TestSerializedHashEmployee < ActiveRecord::Base + self.table_name = "test_employees" + serialize :binary_data, type: Hash, coder: YAML + end @binary_data = "\0\1\2\3\4\5\6\7\8\9" * 10000 @binary_data2 = "\1\2\3\4\5\6\7\8\9\0" * 10000 end @@ -117,6 +121,13 @@ class ::TestEmployee < ActiveRecord::Base expect(@employee.binary_data).to eq(@binary_data) end + it "should find NULL BLOB data when queried with nil" do + TestEmployee.delete_all + TestEmployee.create!(binary_data: nil) + TestEmployee.create!(binary_data: @binary_data) + expect(TestEmployee.where(binary_data: nil)).to have_attributes(count: 1) + end + it "should find serialized NULL BLOB data when queried with nil" do TestEmployee.delete_all TestEmployee.create!(binary_data: nil) diff --git a/spec/active_record/oracle_enhanced/type/text_spec.rb b/spec/active_record/oracle_enhanced/type/text_spec.rb index 837576c7a..c7b235ef9 100644 --- a/spec/active_record/oracle_enhanced/type/text_spec.rb +++ b/spec/active_record/oracle_enhanced/type/text_spec.rb @@ -31,6 +31,10 @@ class ::TestEmployee < ActiveRecord::Base; end class ::Test2Employee < ActiveRecord::Base serialize :comments end + class ::TestSerializedHashEmployee < ActiveRecord::Base + self.table_name = "test_employees" + serialize :comments, type: Hash, coder: YAML + end class ::TestEmployeeReadOnlyClob < ActiveRecord::Base self.table_name = "test_employees" attr_readonly :comments From 33ec70a7c6a1f0c16c7e40b091dfe38428e41bdf Mon Sep 17 00:00:00 2001 From: Brian Thoman Date: Tue, 19 Nov 2024 15:20:40 -0500 Subject: [PATCH 3/7] Slightly adjust the check for nil when comparing LOBs --- lib/arel/visitors/oracle_common.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/arel/visitors/oracle_common.rb b/lib/arel/visitors/oracle_common.rb index 504e01232..b103f4f9f 100644 --- a/lib/arel/visitors/oracle_common.rb +++ b/lib/arel/visitors/oracle_common.rb @@ -9,13 +9,14 @@ module OracleCommon # Fixes ORA-00932: inconsistent datatypes: expected - got CLOB def visit_Arel_Nodes_Equality(o, collector) left = o.left + right = o.right + return super if right.nil? return super unless %i(text binary).include?(cached_column_for(left)&.type) - return super if o.right.nil? # https://docs.oracle.com/cd/B19306_01/appdev.102/b14258/d_lob.htm#i1016668 # returns 0 when the comparison succeeds - comparator = Arel::Nodes::NamedFunction.new("DBMS_LOB.COMPARE", [left, o.right]) + comparator = Arel::Nodes::NamedFunction.new("DBMS_LOB.COMPARE", [left, right]) collector = visit comparator, collector collector << " = 0" collector From 306871db6c26ed782ca0f8dce00654e56096f5cc Mon Sep 17 00:00:00 2001 From: Brian Thoman Date: Tue, 19 Nov 2024 16:52:44 -0500 Subject: [PATCH 4/7] Remove duplicate spec --- spec/active_record/oracle_enhanced/type/binary_spec.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/spec/active_record/oracle_enhanced/type/binary_spec.rb b/spec/active_record/oracle_enhanced/type/binary_spec.rb index b4335dd7b..e30d81365 100644 --- a/spec/active_record/oracle_enhanced/type/binary_spec.rb +++ b/spec/active_record/oracle_enhanced/type/binary_spec.rb @@ -128,13 +128,6 @@ class ::TestSerializedHashEmployee < ActiveRecord::Base expect(TestEmployee.where(binary_data: nil)).to have_attributes(count: 1) end - it "should find serialized NULL BLOB data when queried with nil" do - TestEmployee.delete_all - TestEmployee.create!(binary_data: nil) - TestEmployee.create!(binary_data: @binary_data) - expect(TestEmployee.where(binary_data: nil)).to have_attributes(count: 1) - end - it "should find serialized NULL BLOB data when queried with nil" do TestSerializedHashEmployee.delete_all TestSerializedHashEmployee.create!(binary_data: nil) From 51b7b889fdabc91b73dd1ea11f703df7082081a5 Mon Sep 17 00:00:00 2001 From: Brian Thoman Date: Fri, 6 Dec 2024 21:32:09 -0500 Subject: [PATCH 5/7] Remove new test class after specs run to avoid conflicts in other specs. Also, clean up CLOB specs a bit. --- .../oracle_enhanced/type/binary_spec.rb | 1 + .../oracle_enhanced/type/text_spec.rb | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/spec/active_record/oracle_enhanced/type/binary_spec.rb b/spec/active_record/oracle_enhanced/type/binary_spec.rb index e30d81365..c83a7ae4a 100644 --- a/spec/active_record/oracle_enhanced/type/binary_spec.rb +++ b/spec/active_record/oracle_enhanced/type/binary_spec.rb @@ -26,6 +26,7 @@ class ::TestSerializedHashEmployee < ActiveRecord::Base after(:all) do @conn.drop_table :test_employees, if_exists: true Object.send(:remove_const, "TestEmployee") + Object.send(:remove_const, "TestSerializedHashEmployee") end after(:each) do diff --git a/spec/active_record/oracle_enhanced/type/text_spec.rb b/spec/active_record/oracle_enhanced/type/text_spec.rb index c7b235ef9..e4cf15622 100644 --- a/spec/active_record/oracle_enhanced/type/text_spec.rb +++ b/spec/active_record/oracle_enhanced/type/text_spec.rb @@ -51,6 +51,7 @@ class ::TestSerializeEmployee < ActiveRecord::Base @conn.drop_table :test_serialize_employees, if_exists: true Object.send(:remove_const, "TestEmployee") Object.send(:remove_const, "Test2Employee") + Object.send(:remove_const, "TestSerializedHashEmployee") Object.send(:remove_const, "TestEmployeeReadOnlyClob") Object.send(:remove_const, "TestSerializeEmployee") ActiveRecord::Base.clear_cache! @@ -254,13 +255,20 @@ class ::TestSerializeEmployee < ActiveRecord::Base end it "should find serialized NULL CLOB data when queried with nil" do - Test2Employee.delete_all - Test2Employee.create!(comments: nil) - Test2Employee.create!(comments: { some: 'text' }) - expect(Test2Employee.where(comments: nil)).to have_attributes(count: 1) + TestSerializeEmployee.delete_all + TestSerializeEmployee.create!(comments: nil) + TestSerializeEmployee.create!(comments: { some: 'text' }) + expect(TestSerializeEmployee.where(comments: nil)).to have_attributes(count: 1) end - it "should find serialized NULL CLOB data when queried with {}" do + it "should find serialized Hash NULL CLOB data when queried with nil" do + TestSerializedHashEmployee.delete_all + TestSerializedHashEmployee.create!(comments: nil) + TestSerializedHashEmployee.create!(comments: { some: 'text' }) + expect(TestSerializedHashEmployee.where(comments: nil)).to have_attributes(count: 1) + end + + it "should find serialized Hash NULL CLOB data when queried with {}" do TestSerializedHashEmployee.delete_all TestSerializedHashEmployee.create!(comments: nil) TestSerializedHashEmployee.create!(comments: { some: 'text' }) From 5f07e232028cb187b4301cde8155f58dae4f2a12 Mon Sep 17 00:00:00 2001 From: Brian Thoman Date: Fri, 6 Dec 2024 21:40:34 -0500 Subject: [PATCH 6/7] Update BLOB specs to match CLOB specs in text_spec --- .../oracle_enhanced/type/binary_spec.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/spec/active_record/oracle_enhanced/type/binary_spec.rb b/spec/active_record/oracle_enhanced/type/binary_spec.rb index c83a7ae4a..99d243acc 100644 --- a/spec/active_record/oracle_enhanced/type/binary_spec.rb +++ b/spec/active_record/oracle_enhanced/type/binary_spec.rb @@ -15,6 +15,10 @@ end class ::TestEmployee < ActiveRecord::Base end + class ::TestSerializedEmployee < ActiveRecord::Base + self.table_name = "test_employees" + serialize :binary_data, coder: YAML + end class ::TestSerializedHashEmployee < ActiveRecord::Base self.table_name = "test_employees" serialize :binary_data, type: Hash, coder: YAML @@ -26,6 +30,7 @@ class ::TestSerializedHashEmployee < ActiveRecord::Base after(:all) do @conn.drop_table :test_employees, if_exists: true Object.send(:remove_const, "TestEmployee") + Object.send(:remove_const, "TestSerializedEmployee") Object.send(:remove_const, "TestSerializedHashEmployee") end @@ -130,13 +135,20 @@ class ::TestSerializedHashEmployee < ActiveRecord::Base end it "should find serialized NULL BLOB data when queried with nil" do + TestSerializedEmployee.delete_all + TestSerializedEmployee.create!(binary_data: nil) + TestSerializedEmployee.create!(binary_data: { data: 'some data' }) + expect(TestSerializedEmployee.where(binary_data: nil)).to have_attributes(count: 1) + end + + it "should find serialized Hash NULL BLOB data when queried with nil" do TestSerializedHashEmployee.delete_all TestSerializedHashEmployee.create!(binary_data: nil) TestSerializedHashEmployee.create!(binary_data: { data: 'some data' }) expect(TestSerializedHashEmployee.where(binary_data: nil)).to have_attributes(count: 1) end - it "should find serialized NULL BLOB data when queried with {}" do + it "should find serialized Hash NULL BLOB data when queried with {}" do TestSerializedHashEmployee.delete_all TestSerializedHashEmployee.create!(binary_data: nil) TestSerializedHashEmployee.create!(binary_data: { data: 'some data' }) From c8afffab30b0668987cd96ca2f22b251794788f3 Mon Sep 17 00:00:00 2001 From: Brian Thoman Date: Fri, 6 Dec 2024 22:04:11 -0500 Subject: [PATCH 7/7] Fix Rubocop offenses --- spec/active_record/oracle_enhanced/type/binary_spec.rb | 6 +++--- spec/active_record/oracle_enhanced/type/text_spec.rb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/active_record/oracle_enhanced/type/binary_spec.rb b/spec/active_record/oracle_enhanced/type/binary_spec.rb index 99d243acc..244af7c4b 100644 --- a/spec/active_record/oracle_enhanced/type/binary_spec.rb +++ b/spec/active_record/oracle_enhanced/type/binary_spec.rb @@ -137,21 +137,21 @@ class ::TestSerializedHashEmployee < ActiveRecord::Base it "should find serialized NULL BLOB data when queried with nil" do TestSerializedEmployee.delete_all TestSerializedEmployee.create!(binary_data: nil) - TestSerializedEmployee.create!(binary_data: { data: 'some data' }) + TestSerializedEmployee.create!(binary_data: { data: "some data" }) expect(TestSerializedEmployee.where(binary_data: nil)).to have_attributes(count: 1) end it "should find serialized Hash NULL BLOB data when queried with nil" do TestSerializedHashEmployee.delete_all TestSerializedHashEmployee.create!(binary_data: nil) - TestSerializedHashEmployee.create!(binary_data: { data: 'some data' }) + TestSerializedHashEmployee.create!(binary_data: { data: "some data" }) expect(TestSerializedHashEmployee.where(binary_data: nil)).to have_attributes(count: 1) end it "should find serialized Hash NULL BLOB data when queried with {}" do TestSerializedHashEmployee.delete_all TestSerializedHashEmployee.create!(binary_data: nil) - TestSerializedHashEmployee.create!(binary_data: { data: 'some data' }) + TestSerializedHashEmployee.create!(binary_data: { data: "some data" }) expect(TestSerializedHashEmployee.where(binary_data: {})).to have_attributes(count: 1) end end diff --git a/spec/active_record/oracle_enhanced/type/text_spec.rb b/spec/active_record/oracle_enhanced/type/text_spec.rb index e4cf15622..d9e2e47f3 100644 --- a/spec/active_record/oracle_enhanced/type/text_spec.rb +++ b/spec/active_record/oracle_enhanced/type/text_spec.rb @@ -257,21 +257,21 @@ class ::TestSerializeEmployee < ActiveRecord::Base it "should find serialized NULL CLOB data when queried with nil" do TestSerializeEmployee.delete_all TestSerializeEmployee.create!(comments: nil) - TestSerializeEmployee.create!(comments: { some: 'text' }) + TestSerializeEmployee.create!(comments: { some: "text" }) expect(TestSerializeEmployee.where(comments: nil)).to have_attributes(count: 1) end it "should find serialized Hash NULL CLOB data when queried with nil" do TestSerializedHashEmployee.delete_all TestSerializedHashEmployee.create!(comments: nil) - TestSerializedHashEmployee.create!(comments: { some: 'text' }) + TestSerializedHashEmployee.create!(comments: { some: "text" }) expect(TestSerializedHashEmployee.where(comments: nil)).to have_attributes(count: 1) end it "should find serialized Hash NULL CLOB data when queried with {}" do TestSerializedHashEmployee.delete_all TestSerializedHashEmployee.create!(comments: nil) - TestSerializedHashEmployee.create!(comments: { some: 'text' }) + TestSerializedHashEmployee.create!(comments: { some: "text" }) expect(TestSerializedHashEmployee.where(comments: {})).to have_attributes(count: 1) end end