fix: read Name, Database and Schema during Procedure import #819
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 4 commits
January 13, 2022 09:20
Update ReadProcedure() to set name, database and schema from Show during import. Remove the RETURN argument from ArgumentsSignature() because it is not part of the unqiue signature of a procedure and will not exist during import unless the ID is updated to include it. Change default value of comment to 'user-defined procedure' and fix corresponding tests.
Refactor ArgumentsSignature(), not only removing the RETURN but also capitalizing the entire output to match the output of the SHOW statement. Refactor unit test for Read to more thoroughly test reading the existing procedure with the minimal initial values provided. Update acceptance test to include testing an import.
alldoami
reviewed
Jan 13, 2022
| @@ -230,7 +230,7 @@ type procedure struct { | |||
| Name sql.NullString `db:"name"` | |||
| SchemaName sql.NullString `db:"schema_name"` | |||
| Text sql.NullString `db:"text"` | |||
| DatabaseName sql.NullString `db:"database_name"` | |||
| DatabaseName sql.NullString `db:"catalog_name"` | |||
There was a problem hiding this comment.
This looks like a mistake?
There was a problem hiding this comment.
For some reason, Snowflake returns the database name in a column called catalog_name when calling the SHOW PROCEDURES query.
https://docs.snowflake.com/en/sql-reference/sql/show-procedures.html#output
There was a problem hiding this comment.
well that's weird and not uniform...
|
/ok-to-test sha=4640860 |
|
Integration tests success for 4640860 |
|
/ok-to-test sha=1ef9ac4 |
alldoami
approved these changes
Jan 18, 2022
|
Integration tests success for 1ef9ac4 |
daniepett
pushed a commit
to daniepett/terraform-provider-snowflake
that referenced
this pull request
Feb 9, 2022
This was referenced Nov 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update ReadProcedure() to set name, database and schema during import.
Refactor ArgumentsSignature() to both remove the RETURN portion and to
uppercase the output. The RETURN is technically not part of the unique signature
of a procedure and will not exist when performing an import unless the ID
is updated to include it. Uppercasing the output means the signature will match
the output of the Snowflake SHOW command.
Change default value of comment to
user-defined procedure.Refactor unit test for Read to more thoroughly test reading an existing procedure
with the minimal initial values provided as would be the case during an import.
Update acceptance test to include testing an import.
Test Plan
References
"stored procedures are identified and resolved by the combination of the name and argument types"