Conversation
Expand the converter interface with QValueFromBsonValue, which dispatches on the BSON type to per-type converters. No pre-existing behaviour change. NOTE: This is a split up from a feature branch (#4774) where all pieces are integrated.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
|
|
||
| case bsoncore.TypeTimestamp: | ||
| t, i := v.Timestamp() | ||
| return c.QValueJSONFromTimestamp(t, i), nil |
There was a problem hiding this comment.
Note that bsoncore.TypeTimestamp is not a direct translation to timestamp but https://www.mongodb.com/docs/manual/reference/bson-types/#timestamps
With an extra field for sequential order within the same second.
|
@claude fix linting issues |
|
Claude finished @pfcoperez's task in 1m 43s —— View job Lint fixes pushed
What CI reportedAll three failures were in
Changes- {desc: "Object", input: bson.D{{Key: "x", Value: int64(1)}, {Key: "y", Value: bson.A{}}}, expected: types.QValueJSON{Val: `{"x":1,"y":[]}`}},
+ {
+ desc: "Object",
+ input: bson.D{{Key: "x", Value: int64(1)}, {Key: "y", Value: bson.A{}}},
+ expected: types.QValueJSON{Val: `{"x":1,"y":[]}`},
+ },- require.True(t, math.IsNaN(value) == math.IsNaN(result.Value().(float64)))
- require.True(t, math.IsInf(value, 0) == math.IsInf(result.Value().(float64), 0))
+ require.Equal(t, math.IsNaN(value), math.IsNaN(result.Value().(float64)))
+ require.Equal(t, math.IsInf(value, 0), math.IsInf(result.Value().(float64), 0))Formatting-only and assertion-form-only; no test semantics changed. I also scanned both changed files for any remaining lines over the 144-char Not verified locally: |
- Split the "Object" table entry across lines to satisfy lll (144 cols) - Use require.Equal instead of require.True on a comparison (testifylint) Co-authored-by: Pablo Francisco Pérez Hidalgo <273379+pfcoperez@users.noreply.github.com>
|
@claude fix linting issues |
|
Claude finished @pfcoperez's task in 1m 26s —— View job No lint issues remain — the failure is an infra flake
What the latest lint run actually reportedRun 34870500962 (on That is a transient checksum-database fetch failure against The earlier real issues ( What I pushedSince a re-run needed a push, I bundled a genuine (if tiny) fix rather than an empty commit — a duplicated word in a comment added by this PR: - // Nested documents are are encoded as `QValueJSON` ...
+ // Nested documents are encoded as `QValueJSON` ...This should clear the lint job on the next run assuming the module proxy behaves. Also worth knowing
Not verified locally: |
Co-authored-by: Pablo Francisco Pérez Hidalgo <273379+pfcoperez@users.noreply.github.com>
|
|
08f95af to
25ae3a4
Compare
…onValue Remove the QValueXFromY methods that were only called from the type dispatch switch, constructing types.QValueX directly instead. Only QValueStringFromObjectID and QValueStringFromString remain, as they are also used by QValueStringFromId. Co-authored-by: Pablo Francisco Pérez Hidalgo <273379+pfcoperez@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 participants
Expand the converter interface with QValueFromBsonValue, which dispatches on the BSON type to per-type converters.
No pre-existing behaviour change.
NOTE: This is a split up from a feature branch (#4774) where all pieces are integrated.
Part of: https://linear.app/clickhouse/issue/DBI-1096