Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

query "select @rid as @rid, name from ouser" returns error #4752

Closed
habi4ek opened this issue Aug 7, 2015 · 23 comments
Closed

query "select @rid as @rid, name from ouser" returns error #4752

habi4ek opened this issue Aug 7, 2015 · 23 comments
Assignees
Labels
Milestone

Comments

@habi4ek
Copy link

habi4ek commented Aug 7, 2015

I use an override of field because i want receive the same result for different queries.

when i use query "select from ouser", I receive RIDs in field "@Rid"

orientdb {db=test}> select from ouser
----+----+------+------+-------------------------------------------------------------------------+------+-----

| @Rid| @ CLASS|name |password |status|roles

----+----+------+------+-------------------------------------------------------------------------+------+-----
0 |#5:0|OUser |admin |{SHA-256}8C6976E5B5410.....415BDE90|ACTIVE|[1]
1 |#5:1|OUser |reader|{SHA-256}3D0941964AA3EBDCB00CCE....D5AEC7F31090A0FB30|ACTIVE|[1]
2 |#5:2|OUser |writer|{SHA-256}B93006774CBDD4B299389A03...11A909FBA5|ACTIVE|[1]
----+----+------+------+-------------------------------------------------------------------------+------+-----
3 item(s) found. Query executed in 0.008 sec(s).

but if I use query with projections, I receive RIDs in field "RID"

orientdb {db=test}> select @Rid, name from ouser
----+------+----+------

| @ CLASS| rid |name

----+------+----+------
0 |null |#5:0|admin
1 |null |#5:1|reader
2 |null |#5:2|writer
----+------+----+------
3 item(s) found. Query executed in 0.006 sec(s).

so I use override of the field to get a similar result

orientdb {db=test}> select @Rid as @Rid, name from ouser
----+----+------+------

| @Rid| @ CLASS|name

----+----+------+------
0 |#5:0|OUser |admin
1 |#5:1|OUser |reader
2 |#5:2|OUser |writer
----+----+------+------
3 item(s) found. Query executed in 0.005 sec(s).

the top code worked in version 2.0.13 but in version 2.1.0 the query "select @Rid as @Rid, name from ouser" returns error

orientdb {db=test}> select @Rid as @Rid, name from ouser

Error: com.orientechnologies.orient.core.sql.OCommandSQLParsingException: Error on parsing command at position #0: Encountered " <RECORD_ATTRIBUTE> "@
rid "" at line 1, column 16.
Was expecting one of:
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
...
"" ... "" ...
"`" ...

@luigidellaquila
Copy link
Member

Hi @Ivanov-Yuriy

@ field names are not valid aliases in 2.1, but it's actually quite easy to support.
@lvca shall we support it?

@a-unite
Copy link

a-unite commented Aug 7, 2015

What is rather interesting - it works in "old style" with 2.1 too, but only if your database was created in 2.0.X version.
Does it mean that ODB has different logic for different file formats? If file structure changed for 2.1 - may new logic hurt our (just copied to new version, not imported) data files then?
Sorry for off-topic.

@luigidellaquila
Copy link
Member

Hi @a-unite

it was made for backward compatibility. Old databases maintain the old behavior (no strict SQL), while newly created dbs have strict validation enabled by default

@a-unite
Copy link

a-unite commented Aug 7, 2015

@luigidellaquila thank you!
I understand it is safe to move to 2.1 in production then, and re-import after all parser tests are settled.

This issue with @ sign is quite important for us now. I think, that if the first query is still returns '@Rid', then there are big chances, that you will need to have aliases with the same name (@ included) one day, i.e. for API consistency.
Another way to solve this - is to use projections in every other query.

@habi4ek
Copy link
Author

habi4ek commented Aug 7, 2015

Another way to solve this - is to use projections in every other query.

  • or always return service field with sign "@"
  • or never return service field with sign "@"

not important, only the service field would have the same name always

@a-unite
Copy link

a-unite commented Aug 8, 2015

@Ivanov-Yuriy Sorry, my suggestion was kind of sarcastic, yours makes sense, while could raise compatibility issues (might be solved with another config parameter introduced, though).

@luigidellaquila, @lvca just curious - were there reasons, to strip @ from system fields names in projection queries' results?

@gongdo
Copy link

gongdo commented Aug 12, 2015

I have similar question about this.
Assume a class A has several value properties(a to g) and LINK property(link).

I cannot get 'partial' document with @rid property(with @ prefix):
select @rid, @version, e, f, g, link.value as value from A
@ prefix will be stripped out, anything else is good enough.

But I can get partial document like this:
select *, link.Value as value from A fetchplan a:-2 b:-2 c:-2 d:-2
It doesn't strip the @ sign, and works good.

So technically, there's no problem to make partial document as a result.
For now, I can go with second way. However it's inefficient, I think.
Why don't you support both ways. Is there any cases that could be a problem?

+edited:
Oh I found a trick like this:
select *, link.value as value from A fetchplan *:-2 e:0 f:0 g:0 value:0
(select projection; hide all properties but not specific properties)
It's way better than second way, but it's still inefficient style.
The first one is the best for me.

@a-unite
Copy link

a-unite commented Aug 12, 2015

As I understand presence of system field in server answer should tell us, that we have original object (with @rid, @type and other system fields in place) as result.
Absence of such special fields somehow indicates, that we have synthetic answer from the server (projection, in general, is synthetic set of fields).
As for me it should be clear from particular fields, though. E.g. conjunction of @type, @rid and @version fields, as it is formed now. While I, personally, will be glad to have special indicator\answer property, something like @resultType.

Anyway, we are going off track here.
Support of old behavior is essential for us. Improvements are very much welcomed.

@a-unite
Copy link

a-unite commented Sep 14, 2015

Dear @luigidellaquila, @lvca haven't you come with any decision on the original issue?

@habi4ek
Copy link
Author

habi4ek commented Sep 18, 2015

for me very important understand, this behaviour will stay the same and I must change my code or will you change this behaviour?
please answer me, thank you

@lvca lvca added the bug label Sep 21, 2015
@lvca lvca added this to the 2.1.x (next hotfix) milestone Sep 21, 2015
@luigidellaquila
Copy link
Member

@lvca I need your comment on this, there is no technical reason to allow or deny this behavior.
It's just a matter of consistency, shall we allow fields with @ in projections?

@a-unite
Copy link

a-unite commented Sep 22, 2015

Look guys, it was strange to have different results for projection and "natural" query before 2.1
Example from the initial issue looks weird:
select @rid as @rid, name from ouser
Now it is getting even more strange, when even as @rid doesn't work....

Why don't we return field names as is for new (strict) requests mode? I hope we could use that chance to add good sense to the syntax, not vice versa.

@a-unite
Copy link

a-unite commented Oct 19, 2015

Could you please tell if you are going to come or not with your verdict for this issue anytime soon?
Any answer will be accepted.
Our development cycle is stuck with this matter. Normally we use any workaround which have a common sense or logic behind it, but here situation is quite binary.

@smolinari
Copy link
Contributor

The right solution is to do whatever it takes to be consistent.

Scott

@a-unite
Copy link

a-unite commented Oct 20, 2015

Scott,
please, don't stop here.
What solution do you think is best for consistency?

@smolinari
Copy link
Contributor

Hehe...unfortunately, I am not experienced enough with ODB to give specific suggestions. But, I know from my experience with quality management, being consistent is a core part of being professional.

From my less experienced ODB perspective, the "@" is the problem.

Scott

@a-unite
Copy link

a-unite commented Oct 20, 2015

Thanks.

My thoughts:

  1. Since as @rid was working before - it looks like there are no any dependencies on that kind of field presence in result set for ODB logic.
  2. If one is asking for @ property - we could return it as is, with @ sign, no surprises.
  3. It might be useful to provide some flag in database for next hot-fix version, to switch that behavior on.
  4. This might be default behavior from version 2.2 (flag is on by default for all newly created databases)
  5. Based on statistics, this flag could be omitted completely from version 2.3
  6. Mention that in documentation and keep issues 4 and 5 for upcoming major releases.

Best regards,
Ata

@giastfader
Copy link
Contributor

As a workaround, did you try to unset the strictSql property?

ALTER DATABASE custom strictSQL=false

@a-unite
Copy link

a-unite commented Oct 20, 2015

@giastfader, thanks!
yes we did and this is how our production version works for now.
But I still hope we could switch to current (proper) query parser soon.

@luigidellaquila
Copy link
Member

Hi guys,

I just pushed a fix on branch 2.1.x, could you please verify that it completely fixes your use case?

Thanks

Luigi

ps. please comment but keep this issue open, I have to port the issue to develop

@smolinari
Copy link
Contributor

Hey Luigi.

Thanks for the quick fix. I call that great support. What should we expect to see?

Scott

@luigidellaquila
Copy link
Member

Hi @smolinari

just the fix for the original issue, so the same behavior you had in 2.0.
I'm also planning to return @ alias as is (without removing @), but I have to share it with the team and be sure that nothing breaks

Thanks

Luigi

@a-unite
Copy link

a-unite commented Oct 22, 2015

Hi, @luigidellaquila

Looks like it works as previous (@rid as @rid).
Thank you very much!

You could close this issue, though, I'm still wander if we need aliasing @rid -> rid at all.
Hope you will have chance to discuss and decide on this with team.
Will you open another issue, or we could track this one for the progress with it?

WBR, Ata

@lvca lvca modified the milestones: 2.1.6, 2.1.x (next hotfix) Nov 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

7 participants