Skip to content

Encode query params in binary instead of text #295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jgaskins wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Encode query params in binary instead of text #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0fc1de7
Encode query params in binary instead of text
jgaskins Dec 1, 2024
c8bc7a7
Remove unnecessary code
jgaskins Dec 1, 2024
f636bbf
Handle nil values inside arrays
jgaskins Dec 1, 2024
d05b985
Remove unnecessary specs
jgaskins Dec 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions spec/pg/encoder_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ enum EncoderSpec::Status
end

private def test_insert_and_read(datatype, value, file = __FILE__, line = __LINE__)
it "inserts #{datatype}", file, line do
it "inserts #{value.inspect} as #{datatype}", file, line do
PG_DB.exec "drop table if exists test_table"
PG_DB.exec "create table test_table (v #{datatype})"
PG_DB.exec "insert into test_table values ($1)", args: [value]
Expand All @@ -18,11 +18,18 @@ private def test_insert_and_read(datatype, value, file = __FILE__, line = __LINE
end

describe PG::Driver, "encoder" do
test_insert_and_read "int4", 123
test_insert_and_read "boolean", true
test_insert_and_read "boolean", false

test_insert_and_read "float", 12.34
test_insert_and_read "int2", 123i16
test_insert_and_read "int4", 123i32
test_insert_and_read "int8", 123i64

test_insert_and_read "float4", 12.34f32
test_insert_and_read "float8", 12.34f64
Comment on lines -23 to +29
Copy link
Contributor Author

@jgaskins jgaskins Dec 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I don't specify the sizes of numeric types, I get errors that I don't understand yet. I figured I'd look into it later.

Spec errors 
  1) PG::Driver inserts 123 as int8

       insufficient data left in message (PQ::PQError)
         from src/pq/connection.cr:214:7 in 'handle_error'
         from src/pq/connection.cr:197:7 in 'handle_async_frames'
         from src/pq/connection.cr:173:7 in 'read'
         from src/pq/connection.cr:168:7 in 'read'
         from src/pq/connection.cr:446:31 in 'expect_frame'
         from src/pq/connection.cr:445:5 in 'expect_frame'
         from src/pg/statement.cr:19:5 in 'perform_query'
         from src/pg/statement.cr:35:14 in 'perform_exec'
         from lib/db/src/db/statement.cr:93:9 in 'perform_exec_and_release'
         from lib/db/src/db/statement.cr:78:7 in 'exec:args'
         from lib/db/src/db/pool_statement.cr:19:30 in 'exec:args'
         from lib/db/src/db/query_methods.cr:275:7 in 'exec:args'
         from spec/pg/encoder_spec.cr:12:5 in '->'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/example.cr:50:13 in 'internal_run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/example.cr:38:16 in 'run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/context.cr:20:23 in 'internal_run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/context.cr:346:14 in 'run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/context.cr:20:23 in 'internal_run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/context.cr:155:7 in 'run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/dsl.cr:237:7 in 'execute_examples'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/dsl.cr:220:13 in '->'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/crystal/at_exit_handlers.cr:14:19 in 'run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/crystal/main.cr:53:14 in 'exit'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/crystal/main.cr:48:5 in 'main'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/crystal/main.cr:130:3 in 'main'


  2) PG::Driver inserts 123 as int2

       incorrect binary data format in bind parameter 1 (PQ::PQError)
         from src/pq/connection.cr:214:7 in 'handle_error'
         from src/pq/connection.cr:197:7 in 'handle_async_frames'
         from src/pq/connection.cr:173:7 in 'read'
         from src/pq/connection.cr:168:7 in 'read'
         from src/pq/connection.cr:446:31 in 'expect_frame'
         from src/pq/connection.cr:445:5 in 'expect_frame'
         from src/pg/statement.cr:19:5 in 'perform_query'
         from src/pg/statement.cr:35:14 in 'perform_exec'
         from lib/db/src/db/statement.cr:93:9 in 'perform_exec_and_release'
         from lib/db/src/db/statement.cr:78:7 in 'exec:args'
         from lib/db/src/db/pool_statement.cr:19:30 in 'exec:args'
         from lib/db/src/db/query_methods.cr:275:7 in 'exec:args'
         from spec/pg/encoder_spec.cr:12:5 in '->'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/example.cr:50:13 in 'internal_run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/example.cr:38:16 in 'run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/context.cr:20:23 in 'internal_run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/context.cr:346:14 in 'run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/context.cr:20:23 in 'internal_run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/context.cr:155:7 in 'run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/dsl.cr:237:7 in 'execute_examples'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/spec/dsl.cr:220:13 in '->'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/crystal/at_exit_handlers.cr:14:19 in 'run'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/crystal/main.cr:53:14 in 'exit'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/crystal/main.cr:48:5 in 'main'
         from /opt/homebrew/Cellar/crystal/1.14.0_1/share/crystal/src/crystal/main.cr:130:3 in 'main'


test_insert_and_read "varchar", "hello world"
test_insert_and_read "text", "hello world"

test_insert_and_read "timestamp", Time.utc(2015, 2, 3, 17, 15, nanosecond: 13_000_000)
test_insert_and_read "timestamp", Time.utc(2015, 2, 3, 17, 15, 13, nanosecond: 11_000_000)
Expand All @@ -32,18 +39,24 @@ describe PG::Driver, "encoder" do
test_insert_and_read "int4", EncoderSpec::Status::Open
test_insert_and_read "int4", EncoderSpec::Status::Closed

test_insert_and_read "bool[]", [] of Bool
test_insert_and_read "bool[]", [true]
test_insert_and_read "bool[]", [false]
test_insert_and_read "bool[]", [true, false, true]

test_insert_and_read "float[]", [1.2, 3.4, 5.6]

test_insert_and_read "integer[]", [] of Int32
test_insert_and_read "integer[]", [1, 2, 3]
test_insert_and_read "integer[]", [[1, 2], [3, 4]]
test_insert_and_read "integer[][]", [[1, 2], [3, 4]]
test_insert_and_read "integer[][][]", [[[1, 2], [3, 4]], [[5, 6], [7, 8]]]
Comment on lines 49 to +52
Copy link
Contributor Author

@jgaskins jgaskins Dec 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually surprised that the integer[] example worked before with a nested array. It still does when encoded as binary, too, but I figured I would make this more explicit.


test_insert_and_read "text[]", ["t", "f", "t"]
test_insert_and_read "text[]", ["non-nil value", nil]
test_insert_and_read "text[]", [%("a), %(\\b~), %(c\\"d), %(\uFF8F)]
test_insert_and_read "text[]", ["baz, bar"]
test_insert_and_read "text[]", ["foo}"]
test_insert_and_read "text[][]", [["foo", "bar"], ["baz", "quux"]]

test_insert_and_read "interval", PG::Interval.new
test_insert_and_read "interval", PG::Interval.new(days: 400, microseconds: 5000000)
Expand Down
25 changes: 0 additions & 25 deletions spec/pq/param_spec.cr
View file
Open in desktop

This file was deleted.

3 changes: 2 additions & 1 deletion src/pg/decoder.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,8 @@ module PG
end
end

@@decoders = Hash(Int32, PG::Decoders::Decoder).new(ByteaDecoder.new)
# :nodoc:
class_getter decoders = Hash(Int32, PG::Decoders::Decoder).new(ByteaDecoder.new)
Comment on lines -504 to +505
Copy link
Contributor Author

@jgaskins jgaskins Dec 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because I was going to use decoders as the source for oids, but I didn't end up using it. I can probably undo this change.


def self.from_oid(oid)
@@decoders[oid]
Expand Down
Loading