feat(spanner): support for type UUID #2235
Conversation
product-auto-label
bot
added
size: m
alkatrivedi
added
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
gcf-owl-bot
bot
removed
the
owlbot:run
product-auto-label
bot
added
size: l
alkatrivedi
force-pushed
the
uuid-support
branch
11 times, most recently
from
b31705d to
69cf752
Compare
alkatrivedi
force-pushed
the
uuid-support
branch
from
69cf752 to
5738698
Compare
alkatrivedi
added
kokoro:force-run
gcf-owl-bot
bot
removed
the
owlbot:run
yoshi-kokoro
removed
the
kokoro:force-run
sakthivelmanii
added
the
do not merge
| @@ -5170,6 +5245,196 @@ describe('Spanner', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| // TODO: Enable when the uuid feature has been released. | |||
| describe.skip('uuid', () => { | |||
There was a problem hiding this comment.
have you tested this against cloud-devel?
There was a problem hiding this comment.
@sakthivelmanii We will be merging this when the feature is available in prod right ? So ideally we will remove this "skip" from the PR ?
| @@ -814,6 +817,10 @@ function getType(value: Value): Type { | |||
| return {type: 'bool'}; | |||
| } | |||
|
|
|||
| if (uuid.validate(value)) { | |||
| return {type: 'unspecified'}; | |||
There was a problem hiding this comment.
why unspecified here?
There was a problem hiding this comment.
cause we are not sure if the customer has inserted the UUID value in the column of type UUID/String. Hence, we are returning type as unspecified implicitly, but while making gRPC call we are making sure that if the type is unspecified we don't pass param type with the gaxOptions, this way are letting backend to figure out the type
There was a problem hiding this comment.
@sakthivelmanii uuid value can be send for String column as well. So we want to make use of "unspecified" type and let backend decide it.
Ideally we should not have removed the type mapping when backend started suplorting untyped params, but we did not remove this because of breaking changes.
| @@ -814,6 +817,10 @@ function getType(value: Value): Type { | |||
| return {type: 'bool'}; | |||
| } | |||
|
|
|||
| if (uuid.validate(value)) { | |||
| return {type: 'unspecified'}; | |||
There was a problem hiding this comment.
@sakthivelmanii uuid value can be send for String column as well. So we want to make use of "unspecified" type and let backend decide it.
Ideally we should not have removed the type mapping when backend started suplorting untyped params, but we did not remove this because of breaking changes.
| } | ||
| } else if (typeObject.code !== 'TYPE_CODE_UNSPECIFIED') { | ||
| paramTypes[param] = codec.createTypeObject(type); | ||
| } |
There was a problem hiding this comment.
We can club this into single condition
if (
(type.child && typeObject.code === 'ARRAY' && typeObject.arrayElementType?.code !== 'TYPE_CODE_UNSPECIFIED') ||
(!type.child && typeObject.code !== 'TYPE_CODE_UNSPECIFIED')
) {
paramTypes[param] = codec.createTypeObject(type);
}
| @@ -427,6 +427,7 @@ describe('Spanner', () => { | |||
| before(async () => { | |||
| if (IS_EMULATOR_ENABLED) { | |||
| // TODO: add column Float32Value FLOAT32 and FLOAT32Array Array<FLOAT32> while using float32 feature. | |||
There was a problem hiding this comment.
@alkatrivedi Not in scope for this PR but can you work on this TODO sepaartely now that float32 should be available.
This PR contains support for UUID type on Cloud Spanner.