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

Generate SQL templates #1

Merged
merged 38 commits into from
Oct 24, 2022
Merged

Generate SQL templates #1

merged 38 commits into from
Oct 24, 2022

Conversation

martinjunger
Copy link
Contributor

@martinjunger martinjunger commented Sep 23, 2022

Jira: KBC-2928

Sample data/config.json:

{
  "parameters": {
    "backend": "synapse",
    "operation": "importFull",
    "source": "table",
    "columns": [
      "aaa",
      "bbb"
    ],
    "primaryKeys": [
      "aaa",
      "bbb"
    ]
  },
  "action": "generate"
}

Supported combinations (backend, operation, source):

  • synapse, importFull, fileAbs
  • sypanse, importFull, table
  • snowflake, importFull, fileAbs

@martinjunger martinjunger force-pushed the mj-generate-sql-templates branch from 268ed84 to d4aba15 Compare October 17, 2022 13:48
@martinjunger martinjunger force-pushed the mj-generate-sql-templates branch from a49f40a to e9bbd8a Compare October 18, 2022 14:12
@martinjunger martinjunger marked this pull request as ready for review October 19, 2022 13:29
@martinjunger martinjunger requested review from a team, romanbracinik, tomasfejfar and zajca and removed request for a team and romanbracinik October 19, 2022 14:17
@martinjunger martinjunger force-pushed the mj-generate-sql-templates branch from 913ae1d to c1fb666 Compare October 19, 2022 15:36
Copy link
Member

@zajca zajca left a comment

Choose a reason for hiding this comment

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

Jako POC mě to přijde dobrý, nevím jak stema named parametrama. To je dobrý kakánek ten replace, nebylo špatné tak hodit zrovna example co to znamená, protože mě chvilku trvalo než sem pochopil co to znamená že to dělá replace at the end apod.

@@ -1,4 +1,4 @@
FROM php:8-cli
FROM php:7.4-cli
Copy link
Member

Choose a reason for hiding this comment

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

proč 7.4 jestli kvůli csv-options tak bysme měli vydat novů verzi nebo by šlo ignorovat ten requirement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Zatim to nejspis nepujde... Je tam totiz zavislost:

  • php-db-import-export - php8 ready
    • php-csv-options - php8 ready
    • php-csv-db-import - ^7.1

Muzeme to forknout a upravit (nejaky takovy forky uz jsou)...

Dockerfile Outdated Show resolved Hide resolved
Comment on lines 103 to 112
self::assertStringStartsWith(
"IF OBJECT_ID (N'{{ id(destSchemaName) }}.{{ id(stageTableName ~ '_tmp') }}', N'U') " .
"IS NOT NULL DROP TABLE {{ id(destSchemaName) }}.{{ id(stageTableName ~ '_tmp') }}",
$queries[6]['sql']
);
self::assertStringStartsWith(
"IF OBJECT_ID (N'{{ id(destSchemaName) }}.{{ id(stageTableName ~ '_tmp_rename') }}', N'U') IS NOT NULL " .
"DROP TABLE {{ id(destSchemaName) }}.{{ id(stageTableName ~ '_tmp_rename') }}",
$queries[7]['sql']
);
Copy link
Member

Choose a reason for hiding this comment

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

to budem muset ještě nějak pořešit protože toto je ve finally

<<<SQL
COPY INTO [stageSchemaName634ff46baec71046847136].[__temp_stageTableName634ff46baec72821993597]
FROM 'sourceFile1634ff46baec6c521446965'
WITH ( CREDENTIAL=(IDENTITY='Shared Access Signature', SECRET={{ '?' ~ sourceSasSecret }}) )
Copy link
Member

Choose a reason for hiding this comment

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

nejsu si jistý jestli ten name parametr bude fungovat takto, doctrine na to má :param s dvojtečků. A řekl bych, že SQL server používá @param syntax. Možná bych použil nepojmenované parametry 🤔 ale vlastně nevím.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SECRET jsme vyhodili, pac se ted vsude pouziva Managed identity.
Ale aby se secret nevypisoval do query, tak bude vhodne pouzit funkci, napr. secret(...).

Copy link
Member

Choose a reason for hiding this comment

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

mělo by tam imho pak byt SECRET=? a udělat parameter bind v SQL, tak abysme ten secret nelogovali, případně ho v eventě replacnůt za hvězdičky

@martinjunger martinjunger requested a review from zajca October 24, 2022 07:18
Copy link
Member

@zajca zajca left a comment

Choose a reason for hiding this comment

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

U mě dobrý dělá to to co má, asi bych to nehrotil teď.

@martinjunger martinjunger merged commit 6d777a1 into main Oct 24, 2022
@martinjunger martinjunger deleted the mj-generate-sql-templates branch October 25, 2022 09:04
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