You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Datastrore.transaction.select & Datastore.transaction.filter not working properly in GCloud v0.14+ #806
I have tried the following code snippets in v-0.[14,15,16,17,18]
transaction.filter("email = ",email);
.filter works for v[14,15,16] but not v[17,18] with no changes to my code base. In [17,18] the full amount of rows from the table is returned.
transaction.select(["date","account"]);
.select just hasn't worked for any version [14+]; the full amount of columns is always returned. I have not tested .select with any version before [14].
I'm not able to reproduce either failure. We have a suite of tests that run against a real Datastore backend. Here's one that tests filter and another that tests select. Are we not catching a use case similar to yours?
Pinging @pcostell in case there is a deeper reason a filter or select query might not produce the expected results.
@jiminikiz could you provide a test case that puts some data, then shows this failure when queryingn that data? It would be useful since it seems @stephenplusplus is not able to reproduce.
We wrote a function that we use to run queries, it gets exported in node within module, datastore. Here is a snippet of the module we wrote and have been using for months:
With the above snippet, the value of accounts within the callback will be a single item array in GCloud[<16]. For [16+], accounts is an array of every account in the namespace > path.
I think I have spotted the problem. Any modification to a Query object (which is the object returned from createQuery() or what your snippet calls transaction) doesn't alter the state of the original query. Each change returns a new Query object.
Having each query be immutable was always our goal. Even though we did refactor the class and tests recently, we must not have noticed it wasn't working as intended before, and therefore didn't make any notes about breaking changes in the release.
The fix will be overwriting transaction with the result of the modification:
Having each query be immutable was always our goal.
Do you think we should provide an option for creating mutable queries? It seems like @jiminikiz's case, it being immutable is pretty inconvenient, not to mention I don't know if that's a pattern we enforce throughout gcloud-node.
I think we should pick one or the other, and I would prefer mutable. It has historically been immutable... not sure I remember why exactly, but maybe having to do with when we call start while running a query, it alters the user's provided query. Definitely open to going mutable like any other object.
Thanks guys, the work around @stephenplusplus posted (overwriting transaction) works great for now. I'll be on the look out for when query objects do become mutable in a future version.
Activity
stephenplusplus commentedon Aug 17, 2015
Thanks for reporting the bug. We'll look into it asap.
stephenplusplus commentedon Aug 17, 2015
I'm not able to reproduce either failure. We have a suite of tests that run against a real Datastore backend. Here's one that tests filter and another that tests select. Are we not catching a use case similar to yours?
Pinging @pcostell in case there is a deeper reason a filter or select query might not produce the expected results.
pcostell commentedon Aug 17, 2015
There are requirements around filters / what can be selected, however the Datastore backend will fail if these requirements are not met.
pcostell commentedon Aug 17, 2015
@jiminikiz could you provide a test case that puts some data, then shows this failure when queryingn that data? It would be useful since it seems @stephenplusplus is not able to reproduce.
jiminikiz commentedon Aug 17, 2015
We wrote a function that we use to run queries, it gets exported in node within module,
datastore. Here is a snippet of the module we wrote and have been using for months:We usually name the export
Datastore. Here is an example running a query:With the above snippet, the value of
accountswithin the callback will be a single item array in GCloud[<16]. For[16+],accountsis an array of every account in thenamespace > path.The above query like this will return all of the columns no matter what version of GCloud I am using.
jiminikiz commentedon Aug 17, 2015
Would you like to see the resulting payloads for each version?
stephenplusplus commentedon Aug 17, 2015
I think I have spotted the problem. Any modification to a Query object (which is the object returned from
createQuery()or what your snippet callstransaction) doesn't alter the state of the original query. Each change returns a new Query object.Having each query be immutable was always our goal. Even though we did refactor the class and tests recently, we must not have noticed it wasn't working as intended before, and therefore didn't make any notes about breaking changes in the release.
The fix will be overwriting transaction with the result of the modification:
I'm really sorry that we missed this.
callmehiphop commentedon Aug 17, 2015
Do you think we should provide an option for creating mutable queries? It seems like @jiminikiz's case, it being immutable is pretty inconvenient, not to mention I don't know if that's a pattern we enforce throughout gcloud-node.
stephenplusplus commentedon Aug 17, 2015
I think we should pick one or the other, and I would prefer mutable. It has historically been immutable... not sure I remember why exactly, but maybe having to do with when we call
startwhile running a query, it alters the user's provided query. Definitely open to going mutable like any other object.callmehiphop commentedon Aug 17, 2015
Cool, I think I'd prefer it mutable as well. I'll try and put together a PR for that today or tomorrow.
jiminikiz commentedon Aug 18, 2015
Thanks guys, the work around @stephenplusplus posted (overwriting transaction) works great for now. I'll be on the look out for when query objects do become mutable in a future version.
stephenplusplus commentedon Aug 18, 2015
Thanks for coming by and reporting, I'm glad we figured this out. We'll have a release out today.
(Fixed in #808)
build: add Node 8 tests (#806)
chore: drop dependency on through2 (#806)
chore: remove api-logging teams (#806)
chore: remove api-logging teams (#806)
feat: add Blunderbuss config (#806)
build(auth): add getToken function for the usage of GoogleToken (#806)
fix(deps): update dependency dot-prop to v6 (#806)
fix(deps): update dependency dot-prop to v6 (#806)
fix(deps): update dependency dot-prop to v6 (#806)
fix(deps): update dependency dot-prop to v6 (googleapis#806)