fix(parser): call typeCast for NULL values in the binary protocol (#3368)#4394
Merged
sidorares merged 5 commits intoJul 19, 2026
Merged
Conversation
…dorares#3368) The binary row parser (execute) short-circuited NULL columns to `null` before the user-supplied typeCast could run, so typeCast never saw NULL values. The text row parser (query) has no such short-circuit and passes NULL columns through typeCast, so the two protocols behaved differently. Route NULL columns through typeCast when it is a function, using a null-safe field wrapper whose accessors return null without reading from the packet (a NULL value carries no bytes in the binary row). Applied to both the eval and disableEval (static) binary parsers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4394 +/- ##
=======================================
Coverage 91.52% 91.52%
=======================================
Files 91 91
Lines 14768 14823 +55
Branches 1948 1957 +9
=======================================
+ Hits 13516 13567 +51
- Misses 1252 1256 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
wellwelwel
requested changes
Jul 13, 2026
Collaborator
|
Thanks @Develop-KIM! I left a few revisions. If you need any help, feel free to reach out 🙋🏻♂️ |
Drop the try/finally around the it() blocks -- the connection is created and ended at the describe scope, so poku still closes it if a test fails -- and move the trailing assertion comments into assertion messages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Thanks! Took care of the rest: dropped the try/finally and moved the assertion notes into messages. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3368.
execute()never ran a usertypeCaston NULL columns: the binary row parser set them tonullbefore typeCast could see them, whilequery()(text parser) passes NULL columns through typeCast. So the sametypeCastbehaved differently between the two protocols.This routes NULL columns through
typeCastwhen it's a function, matching the text parser. The wrapper handed totypeCastfor a NULL is null-safe — itsstring()/buffer()/geometry()returnnullwithout reading the packet, since a NULL value carries no bytes in the binary row. WhentypeCastisn't a function (or isfalse), NULL staysnullas before.Applied to both the eval (
binary_parser.js) anddisableEval(static_binary_parser.js) paths.@wellwelwel pointed at the same spot in the issue — thanks for the root-cause note.
Added
test/integration/connection/test-typecast-null-execute.test.mts: it fails onmaster(typeCast skips the NULL column) and passes with the fix, and it also checks that a column following a NULL still decodes correctly (no packet desync). Full suite is green in both eval andSTATIC_PARSER=1modes.