Skip to content

Escaping / (forward slash) causes SQL errors - #329

Merged
phpnode merged 2 commits into
codemix:masterfrom
Ziink:patch-1
Jun 16, 2015
Merged

Escaping / (forward slash) causes SQL errors#329
phpnode merged 2 commits into
codemix:masterfrom
Ziink:patch-1

Conversation

@Ziink

@Ziink Ziink commented Jun 12, 2015

Copy link
Copy Markdown
Contributor

I couldn't figure out if there was a reason that a leading / (forward slash) is being escaped. However, because of this, trying to save a value that begins with a forward slash causes a lexical error in the SQL. (OreintDB 2.1 rc3).

I couldn't figure out if there was a reason that a leading / (forward slash) is being escaped. However, because of this, trying to save a value that begins with a forward slash causes a lexical error in the SQL. (OreintDB 2.1 rc3).
@dmarcelino

Copy link
Copy Markdown
Contributor

Hi @Ziink, your change breaks the travis build: https://travis-ci.org/codemix/oriento/builds/66614890. Can you please take a look?

@Ziink

Ziink commented Jun 15, 2015

Copy link
Copy Markdown
Contributor Author

Seems to me that the test is wrong. Statements such as the following give the expected results.

db.exec('insert into v set val = "///TE /// ST \\\"')
db.exec('select from v where val = "///TE /// ST \\\"')

Why did the first forward slash ever need to be escaped? Besides since the string is ultimately used in Java, forward slash does not need to be escaped and preceding it by a backslash just causes an error.

@phpnode

phpnode commented Jun 15, 2015

Copy link
Copy Markdown
Member

@Ziink various versions of OrientDB require the forward slash to be escaped in some circumstances. Worth trying this query against the actual database to see for yourself, right now the test isn't actually executing it.

@Ziink

Ziink commented Jun 15, 2015

Copy link
Copy Markdown
Contributor Author

That's what I meant by the two statements giving expected results. I verified that the record got written into DB and that I could retrieve it.

I tested on OrientDB 2.1 rc3

@phpnode

phpnode commented Jun 15, 2015

Copy link
Copy Markdown
Member

@Ziink sure, I can believe it's fixed in 2.1, but oriento also supports 2.0 and 1.7, that's why i suggest changing the test so that it hits the db and we inspect the value we get back, that way we can decide whether the test is wrong or whether it's a BC issue.

Added tests that inserts and reads records with fields starting with a slash.
@Ziink

Ziink commented Jun 15, 2015

Copy link
Copy Markdown
Contributor Author

Sorry, my bad. Not familiar with PR protocol.

Anyway, removed the bad test and added a couple of tests that actually test it against the database.

phpnode added a commit that referenced this pull request Jun 16, 2015
Escaping / (forward slash) causes SQL errors
@phpnode
phpnode merged commit addffe3 into codemix:master Jun 16, 2015
@phpnode

phpnode commented Jun 16, 2015

Copy link
Copy Markdown
Member

@Ziink thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants