nodejs-datastore icon indicating copy to clipboard operation
nodejs-datastore copied to clipboard

Still not able to exclude nested entity long string property from indexes with lib verision 6.6.2

Open koteisaev opened this issue 3 years ago • 1 comments

Environment details

  • OS: n/a
  • Node.js version:14.18.1
  • npm version:8.3.0
  • google-cloud-node version: @google-cloud/[email protected]

Steps to reproduce

  1. require google-cloud
  2. create Datastore client for a project
  3. prepare entity properties as list of property name/value/excludeFromIndexes records
  4. call save with an entity having some nested entity with string known to be longer to 1500 bytes
{
"name": "some string",
"email" : "[email protected]",
"bio": { "description": "Loooong string here",
"speciality":"3D Designer"
}
}

Result: error 3 INVALID_ARGUMENT about long property of nested entity

What had been tried so far:

  1. saving entity as list of property/value/excludeFromIndexes applied to root property
  2. saving the root entity as both list of property records and the list of excluded properties with full paths like bio.description as values for this excludeFromIndexes property
  3. saving the root entity as both list of property records as the list of excluded properties with wildcards like bio.*.
  4. saving entity as list of records with value, name, excludeFromIndexes, where value follow the Value structures described here https://cloud.google.com/datastore/docs/reference/data/rest/v1/projects/runQuery#Value No success, always got that error 3 INVALID_ARGUMENT Side note: there is NO official example so far that would explain how to exclude a long nested property in context of nodejs, e. g. here: https://cloud.google.com/datastore/docs/concepts/entities

This issue is becoming a show-stopper that would force us eventually (not today, but at some time point in not so far future) to migrate away from Datastore, if solution would not be found. Migrating to python or other language where official library supports this (exclusion of nested entities properties from indexes) properly is not an option - too time consuming. Writing brand new client lib which would use raw Value data structures as input is not an option - this new code will not be production-ready. Please either give a working example both for case of property with nested entity and for case with property with array of nested entities, or make sure this fix is available via public NPM release like 6.6.3 if fix is done already.... I sure not alone who faced that problem.

koteisaev avatar Dec 22 '21 07:12 koteisaev

I tried to make a fix in a fork, but I not sure whether I understood properly the low-level data format for JSON API, even after all existing tests and few my own work as expected (at least I did not broke something so far for tests where new wrapValue not used?). Fix is in fork https://github.com/SakartveloSoft/nodejs-datastore at DatastoreValueAPI branch. What sort of tests I should add and to which test sets to make sure it all work as expected? Scenarios I want to ensure working are following:

  • nested entity property excluded from indexes - all scalar properties of nested entity must be excluded from indexes
  • array property of nested entities excluded from indexes - all entities with their properties must be excluded from indexes
  • nested entity property, entity with long string (surely longer from 1500 bytes) - that property should be excluded automatically if new API wrapValue used
  • nested array properties, with entities with a long string property - this property of nested entity excluded automatically if wrapValue used.
  • all values within data tree, either nested entity or array of nested entities, wrapped with wrapEntity(value, true) is fully excluded to any possible way from indexes, but saved to DS.

koteisaev avatar Dec 23 '21 13:12 koteisaev

Just to make sure that we are aligned, do you mind posting the code snippet that actually does the steps that are listed under Steps to reproduce?

danieljbruce avatar Jan 31 '23 16:01 danieljbruce

I looked at the fork and I see 2 test cases. If we have a full set of coded test cases with expected behaviour for all the situations you have described then we could consider making some changes.

danieljbruce avatar Mar 01 '23 22:03 danieljbruce

Seems like there are some tests at https://github.com/googleapis/nodejs-datastore/blob/d854b57ed2b9b9f9ea148daedeb6d64e4d99de4d/system-test/datastore.ts#L130 (and below)

vicb avatar Mar 01 '23 22:03 vicb

Do the tests address all the issues the reporter of the issue mentioned? I guess they likely don't because if they did then they would be passing in integration tests and there wouldn't be any issues. I am looking for a suite of tests that don't pass, but that describe the behaviour the user wishes to have. Feel free to reopen the issue when such tests are provided.

danieljbruce avatar Mar 02 '23 19:03 danieljbruce