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

fix: extended query anonymizer tests with functional tests queries #7480

Merged
merged 2 commits into from
May 13, 2021

Conversation

tolgadur
Copy link
Member

@tolgadur tolgadur commented May 6, 2021

Description

This tests take queries used in the ksqldb functional tests to test the QueryAnonymizer and make it more robust before shipping. The issues identified and fixed are:

  • Types weren't anonymized correctly (i.e. custom types weren't anonymized at all)
  • PRIMARY KEYS / KEYS were omitted in the anonymized query
  • Explain queries weren't anonymized correctly. I.e. EXPLAIN SELECT * FROM my_table; just resulted in EXPLAIN query;
  • If the same name was used to create a table and stream name they used to result in the same name. (not sure if this is expected behavior?)
  • SELECT statements werent anonymized at all.
  • * was completely omitted in the anonymizations. i.e. SELECT * FROM my_table
  • JOIN statements were completely ommited in anonymizations.

Unfortunately with the current implementation I did not find a way to determine whether it's a stream or table in SELECT statements. Thus, the anonymizer is just using the generic 'source'. I.e. SELECT * FROM my_table becomes SELECT * FROM source1.

I was also not able to correctly determine the keys in JOIN statements. Thus, the anonymizer is just using the generic key1=key2. I.e.

INSERT INTO OUTPUT SELECT col1, col2, col3  FROM SOURCE1 S1 JOIN SOURCE2 S2 WITHIN 1 SECOND ON col1.k=col2.k;

becomes

INSERT INTO OUTPUT SELECT col1, col2, col3 FROM SOURCE1 S1 JOIN SOURCE2 S2 WITHIN 1 SECOND ON col1.k=col2.k;

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 #")

@tolgadur tolgadur requested a review from swist May 6, 2021 12:16
@ghost
Copy link

ghost commented May 6, 2021

@confluentinc It looks like @tolgadur just signed our Contributor License Agreement. 👍

Always at your service,

clabot

@tolgadur tolgadur force-pushed the extend-query-anonymizer-tests branch 2 times, most recently from e97a32e to 53bf0f7 Compare May 6, 2021 19:29
@tolgadur tolgadur closed this May 10, 2021
@tolgadur tolgadur reopened this May 10, 2021
@tolgadur tolgadur marked this pull request as ready for review May 10, 2021 16:58
@tolgadur tolgadur requested a review from a team as a code owner May 10, 2021 16:58
@tolgadur tolgadur force-pushed the extend-query-anonymizer-tests branch 2 times, most recently from 6e44336 to a1868d8 Compare May 10, 2021 18:30
@tolgadur tolgadur requested a review from swist May 10, 2021 18:36
@tolgadur tolgadur force-pushed the extend-query-anonymizer-tests branch 5 times, most recently from f0886c4 to c781ee8 Compare May 11, 2021 11:02
@tolgadur tolgadur force-pushed the extend-query-anonymizer-tests branch from c781ee8 to 3b63794 Compare May 11, 2021 13:08
Copy link
Member

@swist swist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes

@tolgadur tolgadur merged commit 9a67191 into confluentinc:master May 13, 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.

None yet

2 participants