Skip to content

Conversation

@the-hotmann
Copy link

@jackc Could you please rerun all tests and benchmarks to confirm there are no flaky results?
I’ve applied a lot of optimizations, so the package should now have significantly fewer allocations.

Additionally, I included a number of other improvements.

I skipped many that would degrade DX and have not touched the structure of the structs, even though I think there is a lot to improve:

./pgx-new/internal/sanitize/sanitize.go:204:15: 24 bytes saved: struct with 56 pointer bytes could be 32
./pgx-new/pgconn/internal/bgreader/bgreader.go:18:15: 8 bytes saved: struct with 40 pointer bytes could be 32
./pgx-new/pgproto3/backend.go:11:14: 32 bytes saved: struct with 448 pointer bytes could be 416
./pgx-new/pgproto3/close.go:9:12: 8 bytes saved: struct with 16 pointer bytes could be 8
./pgx-new/pgproto3/copy_both_response.go:13:23: 8 bytes saved: struct with 16 pointer bytes could be 8
./pgx-new/pgproto3/copy_in_response.go:13:21: 8 bytes saved: struct with 16 pointer bytes could be 8
./pgx-new/pgproto3/copy_out_response.go:13:22: 8 bytes saved: struct with 16 pointer bytes could be 8
./pgx-new/pgproto3/describe.go:9:15: 8 bytes saved: struct with 16 pointer bytes could be 8
./pgx-new/pgproto3/error_response.go:9:20: 24 bytes saved: struct with 264 pointer bytes could be 240
./pgx-new/pgproto3/error_response.go:224:22: 24 bytes saved: struct with 280 pointer bytes could be 256
./pgx-new/pgproto3/error_response.go:277:10: 24 bytes saved: struct with 280 pointer bytes could be 256
./pgx-new/pgproto3/frontend.go:12:15: 8 bytes saved: struct of size 1056 could be 1048
./pgx-new/pgproto3/function_call.go:11:19: 8 bytes saved: struct of size 64 could be 56
./pgx-new/pgproto3/function_call_response.go:74:22: 8 bytes saved: struct with 24 pointer bytes could be 16
./pgx-new/pgproto3/gss_enc_request.go:41:22: 16 bytes saved: struct with 32 pointer bytes could be 16
./pgx-new/pgproto3/notification_response.go:11:27: 8 bytes saved: struct with 32 pointer bytes could be 24
./pgx-new/pgproto3/notification_response.go:60:22: 8 bytes saved: struct with 48 pointer bytes could be 40
./pgx-new/pgproto3/ssl_request.go:41:22: 16 bytes saved: struct with 32 pointer bytes could be 16
./pgx-new/pgproto3/startup_message.go:15:21: 8 bytes saved: struct with 16 pointer bytes could be 8
./pgx-new/pgproto3/startup_message.go:85:22: 16 bytes saved: struct with 32 pointer bytes could be 16
./pgx-new/pgproto3/trace.go:15:13: 16 bytes saved: struct with 40 pointer bytes could be 24
./pgx-new/pgconn/auth_scram.go:109:18: 8 bytes saved: struct with 200 pointer bytes could be 192
./pgx-new/pgconn/config.go:34:13: 8 bytes saved: struct of size 256 could be 248
./pgx-new/pgconn/config.go:124:21: 16 bytes saved: struct with 32 pointer bytes could be 16
./pgx-new/pgconn/config.go:131:23: 8 bytes saved: struct with 56 pointer bytes could be 48
./pgx-new/pgconn/errors.go:32:14: 16 bytes saved: struct with 248 pointer bytes could be 232
./pgx-new/pgconn/errors.go:82:26: 8 bytes saved: struct with 48 pointer bytes could be 40
./pgx-new/pgconn/errors.go:109:23: 8 bytes saved: struct with 48 pointer bytes could be 40
./pgx-new/pgconn/errors.go:157:18: 8 bytes saved: struct with 32 pointer bytes could be 24
./pgx-new/pgconn/pgconn.go:41:19: 8 bytes saved: struct with 32 pointer bytes could be 24
./pgx-new/pgconn/pgconn.go:76:13: 8 bytes saved: struct of size 1112 could be 1104
./pgx-new/pgconn/pgconn.go:1449:24: 8 bytes saved: struct with 56 pointer bytes could be 48
./pgx-new/pgconn/pgconn.go:1551:19: 24 bytes saved: struct with 128 pointer bytes could be 104
./pgx-new/pgconn/pgconn.go:1566:13: 16 bytes saved: struct with 80 pointer bytes could be 64
./pgx-new/pgconn/pgconn.go:1741:12: 16 bytes saved: struct with 40 pointer bytes could be 24
./pgx-new/pgconn/pgconn.go:1959:19: 16 bytes saved: struct with 64 pointer bytes could be 48
./pgx-new/pgconn/pgconn.go:2043:15: 8 bytes saved: struct with 112 pointer bytes could be 104
./pgx-new/pgconn/pgconn.go:2076:20: 16 bytes saved: struct with 64 pointer bytes could be 48
./pgx-new/pgconn/pgconn.go:2480:41: 16 bytes saved: struct with 40 pointer bytes could be 24
./pgx-new/internal/stmtcache/lru_cache.go:10:15: 24 bytes saved: struct with 56 pointer bytes could be 32
./pgx-new/pgtype/array.go:19:18: 8 bytes saved: struct with 16 pointer bytes could be 8
./pgx-new/pgtype/array_codec.go:327:25: 8 bytes saved: struct with 40 pointer bytes could be 32
./pgx-new/pgtype/composite.go:31:26: 8 bytes saved: struct with 24 pointer bytes could be 16
./pgx-new/pgtype/composite.go:281:29: 8 bytes saved: struct of size 96 could be 88
./pgx-new/pgtype/composite.go:364:27: 24 bytes saved: struct with 80 pointer bytes could be 56
./pgx-new/pgtype/composite.go:460:29: 32 bytes saved: struct with 64 pointer bytes could be 32
./pgx-new/pgtype/composite.go:517:27: 32 bytes saved: struct with 64 pointer bytes could be 32
./pgx-new/pgtype/multirange.go:276:30: 8 bytes saved: struct with 40 pointer bytes could be 32
./pgx-new/batch.go:12:18: 16 bytes saved: struct with 56 pointer bytes could be 40
./pgx-new/conn.go:119:17: 8 bytes saved: struct with 32 pointer bytes could be 24
./pgx-new/copy_from.go:43:20: 16 bytes saved: struct with 40 pointer bytes could be 24
./pgx-new/copy_from.go:74:19: 16 bytes saved: struct with 48 pointer bytes could be 32
./pgx-new/copy_from.go:109:15: 16 bytes saved: struct with 80 pointer bytes could be 64
./pgx-new/derived_types.go:151:22: 24 bytes saved: struct with 104 pointer bytes could be 80
./pgx-new/named_args.go:39:15: 40 bytes saved: struct with 80 pointer bytes could be 40
./pgx-new/rows.go:122:15: 8 bytes saved: struct with 240 pointer bytes could be 232
./pgx-new/rows.go:343:19: 16 bytes saved: struct with 40 pointer bytes could be 24
./pgx-new/rows.go:707:24: 8 bytes saved: struct with 32 pointer bytes could be 24
./pgx-new/tracer.go:23:24: 8 bytes saved: struct with 32 pointer bytes could be 24
./pgx-new/tracer.go:42:26: 16 bytes saved: struct with 72 pointer bytes could be 56
./pgx-new/tracer.go:67:27: 8 bytes saved: struct with 32 pointer bytes could be 24
./pgx-new/tracer.go:86:26: 8 bytes saved: struct with 24 pointer bytes could be 16
./pgx-new/tx.go:152:11: 16 bytes saved: struct with 32 pointer bytes could be 16
./pgx-new/pgxpool/pool.go:27:19: 16 bytes saved: struct with 104 pointer bytes could be 88
./pgx-new/pgxpool/pool.go:78:11: 8 bytes saved: struct of size 224 could be 216
./pgx-new/pgxpool/pool.go:122:13: 8 bytes saved: struct of size 128 could be 120
./pgx-new/tracelog/tracelog.go:137:15: 8 bytes saved: struct with 32 pointer bytes could be 24
./pgx-new/stdlib/sql.go:256:16: 32 bytes saved: struct with 360 pointer bytes could be 328
./pgx-new/stdlib/sql.go:330:13: 8 bytes saved: struct with 16 pointer bytes could be 8
./pgx-new/stdlib/sql.go:417:11: 32 bytes saved: struct with 384 pointer bytes could be 352
./pgx-new/stdlib/sql.go:635:11: 8 bytes saved: struct with 64 pointer bytes could be 56
exit status 1

Anyway feel free to criticize :)

@the-hotmann
Copy link
Author

the-hotmann commented Nov 30, 2025

I also fixed all shadow-imports like "sql" (named it sqlVar - sql variable).

Here: I would like to ask for your attention:

https://github.com/jackc/pgx/blob/master/pgtype/int.go#L1173
https://github.com/jackc/pgx/blob/master/pgtype/int.go#L1176

As for me this seems to be a bug:

	if n.Int64 < math.MinInt64 {
		return fmt.Errorf("%d is less than minimum value for Int8", n.Int64)
	}
	if n.Int64 > math.MaxInt64 {
		return fmt.Errorf("%d is greater than maximum value for Int8", n.Int64)
	}

It checks against math.MinInt64 and math.MaxInt64 which does not make sense, as n.Int64 can never be bigger than math.MaxInt64 and the error text hints it should check for math.MaxInt8.
I have fixed this in my PR - not knowing (but assuming) what actually is right.

@the-hotmann
Copy link
Author

@jackc if you like I can break this PR down into multiple PRs based on the optimizations:

  • Shadow-Import
  • modernization
  • val-copy optimization
  • using pointers on methods

...

if that would help

@jackc
Copy link
Owner

jackc commented Dec 13, 2025

This would definitely need to be broken up into multiple, maybe even many, separate PR's. I did look at it but +1,780 −1,638 is a lot to review at once.

However, I did notice several types of changes that I'm not optimistic about:


Changing loops to use indexes instead of the value. e.g.

for _, bi := range b.QueuedQueries {
	sc.Invalidate(bi.SQL)

to

for i := range b.QueuedQueries {
	sc.Invalidate(b.QueuedQueries[i].SQL)

I find the former easier to read.


Changing benchmarks to use b.N instead of b.Loop(). b.Loop() automatically resets the benchmark timer. So this change makes the benchmark less accurate. I believe b.Loop() is also idiomatic now.


Renaming variables like sql to sqlVar to avoid shadowing. If there's no ambiguity in the function, I don't see the harm in shadowing an import. It reads better.


Changing the function arguments or return values from values to pointers is a breaking change. We can't do that. e.g. GetConnector and OpenDB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants