[native] Advance velox and disable nullif tests.#20037
[native] Advance velox and disable nullif tests.#20037amitkdutta merged 1 commit intoprestodb:masterfrom
Conversation
There was a problem hiding this comment.
@vermapratyush These tests fails with
VeloxUserError: NULLIF not supported natively
Disabling for now to keep the CI green. Shall I delete these tests?
There was a problem hiding this comment.
Let's disable it.
They should still be working, I will have a look.
There was a problem hiding this comment.
@amitkdutta @vermapratyush Would one of you create a GitHub issue to describe the problem? It would be nice to do that every time we disable a test and include a link to the issue in a comment next to disabled test.
amitkdutta
force-pushed
the
adv_velox_61
branch
2 times, most recently
from
929c313 to
64b7397
Compare
xiaoxmeng
left a comment
xiaoxmeng
left a comment
There was a problem hiding this comment.
@amitkdutta LGTM. Could you briefly describe the deleted test in pr description!
amitkdutta
changed the title
|
Is this PR breaking the test? #19791 Should we revert this PR instead? |
|
We can revert this for now. #20031 |
|
Revert PR here: #20038 |
|
Here is a proper fix for the failing test #20039 |
vermapratyush
left a comment
vermapratyush
left a comment
There was a problem hiding this comment.
I have reverted the offending PR here #20038
We don't need to disable the tests now.
There will still be test failure in presto e2e but thats legitimate failure and unhandled in prestissimo.
@vermapratyush Reverting your change does not fix Presto. Lets disable it to keep the CI green, and then debug. Otherwise random failure can go in as its currently already failing. |
@kewang1024 I disable |
5 participants
Uh oh!
There was an error while loading. Please reload this page.