feat: support flexible columns on adapt package #474
Conversation
alvarowolfx
added
owlbot:run
product-auto-label
bot
added
the
api: bigquerystorage
gcf-owl-bot
bot
removed
the
owlbot:run
alvarowolfx
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
product-auto-label
bot
added
size: l
src/adapt/proto.ts
Outdated
| } | ||
| const isNameCompatible = isProtoCompatible(name); | ||
| if (!isNameCompatible) { | ||
| name = generatePlaceholderFieldName(name, fNumber); |
There was a problem hiding this comment.
Do we think name collisions after sanitizing is a significant-enough concern at this point? e.g. foo_👍 and foo_👎 as an example, since both would end up with foo__ if I'm reading correctly.
There was a problem hiding this comment.
yeah, I was concerned about the field collision, but not sure the best approach here. The approach that the Java side took to always convert a non proto compatible field to a format like col_{b64(fieldName), but that generates a field that is not easy to guess or use (like col_a5Bmvz).
And I meant to transform foo_👍 into just foo_, but I'll check again if that is true
There was a problem hiding this comment.
Yeah, it comes down to the consumer model. I believe the expectation in java is they're the ones consuming the schema so they know to check the annotation.
There was a problem hiding this comment.
If you build up a reserved field list while you're walking the list of fields you can add basic conflict handling, but it does highlight that field name resolution necessitates awareness of the annotation.
There was a problem hiding this comment.
I sent some changes to actually work more closely on how the Java client lib going, which is to allow users to write JSON data using flexible column field names and under the hood use the placeholders to convert data.
alvarowolfx
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
Towards internal b/362388647