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

feat: add source statement to SourceDescription #4134

Conversation

stevenpyzhang
Copy link
Member

@stevenpyzhang stevenpyzhang commented Dec 13, 2019

Description

Adds the SQL statement used to create a source to the SourceDescription object that's returned in the rest response.

This makes it easier for users to retrieve a statement for a Stream/Table in case they need to update and recreate the object. Instead of reconstructing the statement from the output of a DESCRIBE (which might not be very convenient with large schemas), they can just copy and paste it from the console/rest response.

The statement field is only present in the CLI console for EXTENDED commands.

This PR is done with the assumption that if a UPDATE/ALTER command is introduced later, getSqlExpression() would return the updated SQL statement that would be able to recreate that source. (Currently it just returns the original statement)

Testing done

Updated console tests
Spun up Server and CLI and checked the print

Name                 : FOSOD
Type                 : STREAM
Statement            : CREATE STREAM fosod(age BIGINT) WITH (KAFKA_TOPIC='foo',  VALUE_FORMAT='JSON');
Key field            : 
Key format           : STRING
Timestamp field      : Not set - using <ROWTIME>
Value format         : JSON
Kafka topic          : foo (partitions: 2, replication: 1)

 Field   | Type                      
-------------------------------------
 ROWTIME | BIGINT           (system) 
 ROWKEY  | VARCHAR(STRING)  (system) 
 AGE     | BIGINT                    
-------------------------------------

Local runtime statistics
------------------------


(Statistics of the local KSQL server interaction with the Kafka topic foo)
[
    {
        "@type": "sourceDescription",
        "statementText": "describe extended fosod;",
        "sourceDescription": {
            "name": "FOSOD",
            "readQueries": [],
            "writeQueries": [],
            "fields": [
                {
                    "name": "ROWTIME",
                    "schema": {
                        "type": "BIGINT",
                        "fields": null,
                        "memberSchema": null
                    }
                },
                {
                    "name": "ROWKEY",
                    "schema": {
                        "type": "STRING",
                        "fields": null,
                        "memberSchema": null
                    }
                },
                {
                    "name": "AGE",
                    "schema": {
                        "type": "BIGINT",
                        "fields": null,
                        "memberSchema": null
                    }
                }
            ],
            "type": "STREAM",
            "key": "",
            "timestamp": "",
            "statistics": "",
            "errorStats": "",
            "extended": true,
            "format": "JSON",
            "topic": "foo",
            "partitions": 2,
            "replication": 1,
            "statement": "CREATE STREAM fosod(age BIGINT) WITH (KAFKA_TOPIC='foo',  VALUE_FORMAT='JSON');"
        },
        "warnings": []
    }
]

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

@stevenpyzhang stevenpyzhang requested a review from a team as a code owner December 13, 2019 22:44
Copy link
Contributor

@big-andy-coates big-andy-coates left a comment

Choose a reason for hiding this comment

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

Thanks @stevenpyzhang, LGTM with the exception of the hashCode omission and the missing test.

@@ -43,6 +43,7 @@
private final String topic;
private final int partitions;
private final int replication;
private final String statement;
Copy link
Contributor

Choose a reason for hiding this comment

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

needs including in hashCode as well.

This highlights the lack of testing of equals and hashCode in this class. Can you add a suitable unit test please? Take a look at our use of EqualsTester, e.g. in CreateStreamTest, for an example of the testing pattern.

@stevenpyzhang stevenpyzhang force-pushed the add-statement-source-description branch from 9bd5db1 to 463a6d6 Compare December 16, 2019 19:35
@stevenpyzhang stevenpyzhang force-pushed the add-statement-source-description branch from 463a6d6 to 39b7232 Compare December 16, 2019 21:16
@stevenpyzhang stevenpyzhang merged commit 1146aa5 into confluentinc:master Dec 16, 2019
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