Handle null::text casts in postgresql dialect, fix panics #479
Conversation
| @@ -8762,15 +8762,6 @@ def test_invalid_specified_values(self): | |||
| self.assertTrue(found, "Not found any expected exception") | |||
|
|
|||
|
|
|||
| class TestPanicRecover(AcraCatchLogsMixin, BaseTestCase): | |||
There was a problem hiding this comment.
it was one known case that caused panics. I don't know other cases to put here *( We can leave this test empty, but IMHO, we can write it one more time when we will need it.
There was a problem hiding this comment.
i thought this PR means that panics won't happen for null:text cast, so the test should always pass now.
There was a problem hiding this comment.
And after this fix it doesn't panic anymore )
There was a problem hiding this comment.
i thought this PR means that panics won't happen for null:text cast, so the test should always pass now.
oh, you wrote it before me)) 10 sec difference) Yes, it fixes and doesn't panic anymore
There was a problem hiding this comment.
okay, thank you for the explanation! great job!
Due to using sqlparser that targeted to MySQL, it didn't use to support postgresql's type castings, especially
null::text.Previously we extended SQLVal to support
::<type>type casts, to avoid a lot of refactorings and re-use existing code that depends on SQLVal (for example our transparent encryption). But it's simplest way to support such type casts. Harder is to refactorsql.yand used SQL's grammar, and propagate casts to all kinds of lexical rules related to SQL values (select expr,functions, etc).But Acra works with simple SQL value literals and don't support complicated SQL values like functions or
(select 1)::text, so for now we don't need to do it and can leave it for later (probably we will switch to another parser earlier than meet real case where it need:)So here all complicated SQL expressions will be stored in
unknownfield ofSQLValto reuse existing code and just serialize/deserialize when it need as is.Checklist
with new changes