V1 XMLCodec supports encoding + scanning XML column type #2083
Conversation
This file contains 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
nickcruess-soda
commented
Jul 11, 2024
|
It looks like the tests will need to skip CockroachDB. |
lyuboxa
reviewed
Sep 11, 2024
| } | ||
|
|
||
| var dst any | ||
| err := c.Unmarshal(src, &dst) |
There was a problem hiding this comment.
@nickcruess-soda @jackc I seem to be late to the game but this will always return nil, since Unmarshal(..) expects the variable to be of slice, struct or string. https://pkg.go.dev/encoding/xml#Unmarshal
Unmarshal parses the XML-encoded data and stores the result in the value pointed to by v,
which must be an arbitrary struct, slice, or string.
Well-formed data that does not fit into v is discarded.
https://go.dev/play/p/yLdFqZPaoUo
Returning the bytes seem to be the better choice.
Am I missing something?
There was a problem hiding this comment.
@lyuboxa I think you're right.
@nickcruess-soda What do you think?
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.
We have a need for the XML column type but observed that it isn't currently supported. I've taken the JSONCodec as a starting point and adapted it for XML. They turned out to be quite similar. I left in some of the issue-referencing comments from the JSONCodec which seemed relevant here as well, such as the one explaining the
driver.Valuervsxml.Marshalerordering inPlanEncode.While I was at it I also noticed some unused code related to the JSONCodec (
scanPlanJSONToBytesScanner), which I removed in the second commit. Happy to drop that if this isn't the place/time for it.