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

test: fix drop statement tests in QTT #7994

Closed
wants to merge 2 commits into from
Closed

test: fix drop statement tests in QTT #7994

wants to merge 2 commits into from

Conversation

jzaralim
Copy link
Contributor

Description

Fixed because it is needed for #7474.

In the drop_sources.json QTT test, all of the drop statements dropped streams that shared a topic with another stream, for example.

"CREATE STREAM input2 (K STRING KEY, data VARCHAR) WITH (kafka_topic='input', value_format='DELIMITED');",
"DROP STREAM IF EXISTS input2;",
"CREATE STREAM input (K STRING KEY, data VARCHAR) WITH (kafka_topic='input', value_format='DELIMITED');",
"CREATE STREAM output AS SELECT * FROM input;"

This is because the QTT would have thrown an error if the dropped source had a topic that was only referenced once.

This PR catches and ignores the error so that we could run DROP tests without forcing streams/tables to share topics:

"CREATE SOURCE STREAM abc (K STRING KEY, data VARCHAR) WITH (kafka_topic='abc', value_format='DELIMITED');",
"CREATE STREAM input (K STRING KEY, data VARCHAR) WITH (kafka_topic='input', value_format='DELIMITED');",
"CREATE STREAM output AS SELECT * FROM input;",
"DROP STREAM abc DELETE TOPIC;"

The DROP statements still doesn't completely work though - the following test runs, but the test code thinks that the topic abc was deleted even though it wasn't.

"CREATE STREAM abc (K STRING KEY, data VARCHAR) WITH (kafka_topic='abc', value_format='DELIMITED');",
"CREATE STREAM input (K STRING KEY, data VARCHAR) WITH (kafka_topic='input', value_format='DELIMITED');",
"CREATE STREAM output AS SELECT * FROM input;",
"DROP STREAM abc;"

However, if we just want to use these tests to check the behavior of DROP vs DROP.... DELETE TOPIC; for SOURCE sources without actually checking the topics, then this is good enough.

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@jzaralim jzaralim requested a review from a team as a code owner August 12, 2021 18:15
@jzaralim jzaralim requested review from spena and mjsax August 12, 2021 18:15
@mjsax
Copy link
Member

mjsax commented Oct 15, 2021

@jzaralim It seems #7474 was merged, so this fix was actually not needed for it?

Not sure if I understand the examples in the PR description? Which statements actually fail, and what it the error? It seems your PR only swallows some exceptions, but I am wondering what the actual root cause of those exceptions is, and if we should rather fix those, to avoid that an exception is thrown incorrectly in the first place?

@jzaralim
Copy link
Contributor Author

@mjsax This change ended up not being needed because the errors that are thrown when running DROP ... DELETE TOPIC on a source table are thrown before the ones here.

The problem here is with the QTT testing framework. The first is that DROP statements always delete the topic even if the DELETE TOPIC clause is not present. This was not fixed here. The second problem is that the QTT keeps track of all topics that have ever existed while the test runs, and in the end, it will throw an error if one of these topics don't exist anymore. That is the problem that is fixed in this PR.

This is not worth fixing unless we really need drop statements to work in QTTs

@jzaralim jzaralim closed this Oct 15, 2021
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.

2 participants