Don't overwrite flags of timestamp coders

It is unexpected, since it makes the coder behave contrary to the class name. Moreover it causes a regression in rails: rails/rails#48049 Fixes ged#524
larskanis · Apr 24, 2023 · 77bd5e8 · 77bd5e8
1 parent 70460e3
commit 77bd5e8
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 9 deletions.
diff --git a/lib/pg/binary_decoder/timestamp.rb b/lib/pg/binary_decoder/timestamp.rb
@@ -7,19 +7,19 @@ module BinaryDecoder
 		class TimestampUtc < Timestamp
 			def initialize(hash={}, **kwargs)
 				warn "PG::Coder.new(hash) is deprecated. Please use keyword arguments instead! Called from #{caller.first}" unless hash.empty?
-				super(flags: PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_UTC, **hash, **kwargs)
+				super(**hash, **kwargs, flags: PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_UTC)
 			end
 		end
 		class TimestampUtcToLocal < Timestamp
 			def initialize(hash={}, **kwargs)
 				warn "PG::Coder.new(hash) is deprecated. Please use keyword arguments instead! Called from #{caller.first}" unless hash.empty?
-				super(flags: PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_LOCAL, **hash, **kwargs)
+				super(**hash, **kwargs, flags: PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_LOCAL)
 			end
 		end
 		class TimestampLocal < Timestamp
 			def initialize(hash={}, **kwargs)
 				warn "PG::Coder.new(hash) is deprecated. Please use keyword arguments instead! Called from #{caller.first}" unless hash.empty?
-				super(flags: PG::Coder::TIMESTAMP_DB_LOCAL | PG::Coder::TIMESTAMP_APP_LOCAL, **hash, **kwargs)
+				super(**hash, **kwargs, flags: PG::Coder::TIMESTAMP_DB_LOCAL | PG::Coder::TIMESTAMP_APP_LOCAL)
 			end
 		end
 	end

diff --git a/lib/pg/binary_encoder/timestamp.rb b/lib/pg/binary_encoder/timestamp.rb
@@ -7,13 +7,13 @@ module BinaryEncoder
 		class TimestampUtc < Timestamp
 			def initialize(hash={}, **kwargs)
 				warn "PG::Coder.new(hash) is deprecated. Please use keyword arguments instead! Called from #{caller.first}" unless hash.empty?
-				super(flags: PG::Coder::TIMESTAMP_DB_UTC, **hash, **kwargs)
+				super(**hash, **kwargs, flags: PG::Coder::TIMESTAMP_DB_UTC)
 			end
 		end
 		class TimestampLocal < Timestamp
 			def initialize(hash={}, **kwargs)
 				warn "PG::Coder.new(hash) is deprecated. Please use keyword arguments instead! Called from #{caller.first}" unless hash.empty?
-				super(flags: PG::Coder::TIMESTAMP_DB_LOCAL, **hash, **kwargs)
+				super(**hash, **kwargs, flags: PG::Coder::TIMESTAMP_DB_LOCAL)
 			end
 		end
 	end

diff --git a/lib/pg/text_decoder/timestamp.rb b/lib/pg/text_decoder/timestamp.rb
@@ -7,19 +7,19 @@ module TextDecoder
 		class TimestampUtc < Timestamp
 			def initialize(hash={}, **kwargs)
 				warn "PG::Coder.new(hash) is deprecated. Please use keyword arguments instead! Called from #{caller.first}" unless hash.empty?
-				super(flags: PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_UTC, **hash, **kwargs)
+				super(**hash, **kwargs, flags: PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_UTC)
 			end
 		end
 		class TimestampUtcToLocal < Timestamp
 			def initialize(hash={}, **kwargs)
 				warn "PG::Coder.new(hash) is deprecated. Please use keyword arguments instead! Called from #{caller.first}" unless hash.empty?
-				super(flags: PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_LOCAL, **hash, **kwargs)
+				super(**hash, **kwargs, flags: PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_LOCAL)
 			end
 		end
 		class TimestampLocal < Timestamp
 			def initialize(hash={}, **kwargs)
 				warn "PG::Coder.new(hash) is deprecated. Please use keyword arguments instead! Called from #{caller.first}" unless hash.empty?
-				super(flags: PG::Coder::TIMESTAMP_DB_LOCAL | PG::Coder::TIMESTAMP_APP_LOCAL, **hash, **kwargs)
+				super(**hash, **kwargs, flags: PG::Coder::TIMESTAMP_DB_LOCAL | PG::Coder::TIMESTAMP_APP_LOCAL)
 			end
 		end
 

diff --git a/spec/pg/type_spec.rb b/spec/pg/type_spec.rb
@@ -523,12 +523,15 @@ def expect_deprecated_coder_init
 			end
 		end
 
-		it "should overwrite default values as hash" do
+		it "should overwrite default format" do
 			t = nil
 			expect_deprecated_coder_init do
 				t = PG::BinaryEncoder::Int4.new({format: 0})
 			end
 			expect( t.format ).to eq( 0 )
+
+			t = PG::BinaryEncoder::Int4.new(format: 0)
+			expect( t.format ).to eq( 0 )
 		end
 
 		it "should take hash argument" do
@@ -559,6 +562,27 @@ def expect_deprecated_coder_init
 			expect( t.name ).to eq( "abcä" )
 		end
 
+		it "shouldn't overwrite timestamp flags" do
+			t = PG::TextDecoder::TimestampUtc.new({flags: PG::Coder::TIMESTAMP_DB_LOCAL})
+			expect( t.flags ).to eq( PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_UTC )
+			t = PG::TextDecoder::TimestampUtcToLocal.new({flags: PG::Coder::TIMESTAMP_APP_UTC})
+			expect( t.flags ).to eq( PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_LOCAL )
+			t = PG::TextDecoder::TimestampLocal.new({flags: PG::Coder::TIMESTAMP_DB_UTC})
+			expect( t.flags ).to eq( PG::Coder::TIMESTAMP_DB_LOCAL | PG::Coder::TIMESTAMP_APP_LOCAL )
+
+			t = PG::BinaryDecoder::TimestampUtc.new({flags: PG::Coder::TIMESTAMP_DB_LOCAL})
+			expect( t.flags ).to eq( PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_UTC )
+			t = PG::BinaryDecoder::TimestampUtcToLocal.new({flags: PG::Coder::TIMESTAMP_APP_UTC})
+			expect( t.flags ).to eq( PG::Coder::TIMESTAMP_DB_UTC | PG::Coder::TIMESTAMP_APP_LOCAL )
+			t = PG::BinaryDecoder::TimestampLocal.new({flags: PG::Coder::TIMESTAMP_DB_UTC})
+			expect( t.flags ).to eq( PG::Coder::TIMESTAMP_DB_LOCAL | PG::Coder::TIMESTAMP_APP_LOCAL )
+
+			t = PG::BinaryEncoder::TimestampUtc.new({flags: PG::Coder::TIMESTAMP_DB_LOCAL})
+			expect( t.flags ).to eq( PG::Coder::TIMESTAMP_DB_UTC )
+			t = PG::BinaryEncoder::TimestampLocal.new({flags: PG::Coder::TIMESTAMP_APP_LOCAL})
+			expect( t.flags ).to eq( PG::Coder::TIMESTAMP_DB_LOCAL )
+		end
+
 		it "should deny changes when frozen" do
 			t = PG::TextEncoder::String.new.freeze
 			expect{ t.format = 1 }.to raise_error(FrozenError)