CT-1128 SNFLK autocast types when using string stage table by zajca · Pull Request #90 · keboola/storage-backend

zajca · 2023-09-19T13:45:11Z

zajca · 2023-09-20T07:18:14Z

packages/php-datatypes/src/Definition/DefinitionInterface.php

+    public function getType(): string;
+
+    public function getLength(): ?string;
+
+    public function isNullable(): bool;
+
+    public function getDefault(): ?string;
+


nechápu proč to neni v tom interface

zajca · 2023-09-20T07:18:44Z

packages/php-db-import-export/src/Backend/Snowflake/SnowflakeImportOptions.php

        bool $nullManipulation = self::NULL_MANIPULATION_ENABLED,
-        array $ignoreColumns = []
+        array $ignoreColumns = [],
+        array $autoCastTypes = [],


to co sa má castovat sem dal jako option ať to jde nastavit z connection

a seznam typu na autoCast budeme drzet v datatypes (jako se to planuje tady Package/StorageBackend/src/LegacyBackendConfig/SnowflakeConfig.php ) nebo to bude drzet connnection tady Package/StorageBackend/src/LegacyBackendConfig/SnowflakeConfig.php ?

plus mohla by ta option dostat kus komentare? co to dela? to je strasnej pain tady tech options, ze pak clovek musi rozmotavat kde se to pouziva a jake to ma dopady

toto je to co bych tam chtel https://github.com/keboola/storage-backend/pull/90/files#r1331117689

Smazal sem to, upravil sem test, vlatně je to potřeba po dalším testování jen pro 3 typy:

Snowflake::TYPE_VARIANT, Snowflake::TYPE_OBJECT, Snowflake::TYPE_ARRAY,

ostatní SNFLK nějak sám přecastuje z toho stringu.
Původně sem nad tím přemýšlal tak, že to půjde nastavit v connection a nebude potřeba šahat sem, ale vlastně je to blbost a chceš to mět tady odzkůšané.

zajca · 2023-09-20T07:20:03Z

packages/php-db-import-export/src/Backend/Snowflake/ToFinalTable/SqlBuilder.php

+                $destinationColumn = $columnMap->getDestination($sourceColumn);
+                $type = $destinationColumn->getColumnDefinition()->getType();
+                $useAutoCast = in_array($type, $importOptions->autoCastTypes(), true);
+                $isSameType = $type === $sourceColumn->getColumnDefinition()->getType();
+                if ($useAutoCast && !$isSameType) {
+                    if ($type === Snowflake::TYPE_OBJECT) {
+                        // object can't be casted from string but can be casted from variant
+                        $columnsSetSql[] = sprintf(
+                            'CAST(TO_VARIANT(%s) AS %s) AS %s',
+                            SnowflakeQuote::quoteSingleIdentifier($sourceColumn->getColumnName()),
+                            $destinationColumn->getColumnDefinition()->getSQLDefinition(),
+                            SnowflakeQuote::quoteSingleIdentifier($destinationColumn->getColumnName())
+                        );
+                        continue;
+                    }
+                    $columnsSetSql[] = sprintf(
+                        'CAST(%s AS %s) AS %s',
+                        SnowflakeQuote::quoteSingleIdentifier($sourceColumn->getColumnName()),
+                        $destinationColumn->getColumnDefinition()->getSQLDefinition(),
+                        SnowflakeQuote::quoteSingleIdentifier($destinationColumn->getColumnName())
+                    );
+                    continue;
+                }
+                $columnsSetSql[] = SnowflakeQuote::quoteSingleIdentifier($sourceColumn->getColumnName());
                continue;


null manipulation flag je pro load/unload do WS pro netypové tabulky když chceme konvertovat '' na null hodnoty.

Pro load z CSV ho je ten flag false a to je kdy to potřebujeme konvertovat ze string stage tabulek do typovaných tabulek.

stejné věc je potřeba pro vkládání nových hodnot a pro update stávajících hodnot.

zajca · 2023-09-20T07:22:31Z

packages/php-db-import-export/src/Backend/SourceDestinationColumnMap.php

+/**
+ * Class will create map of table column based on columns order
+ */
+final class SourceDestinationColumnMap


tady to mapování source->destination sloupců sem potřeboval dostat někam ven.

jirkasemmler

v zasade ok, mam par doplnujicich dotazu a prani

jirkasemmler · 2023-09-22T06:29:49Z

packages/php-db-import-export/src/Backend/Snowflake/SnowflakeImportOptions.php

        bool $nullManipulation = self::NULL_MANIPULATION_ENABLED,
-        array $ignoreColumns = []
+        array $ignoreColumns = [],
+        array $autoCastTypes = [],


a seznam typu na autoCast budeme drzet v datatypes (jako se to planuje tady Package/StorageBackend/src/LegacyBackendConfig/SnowflakeConfig.php ) nebo to bude drzet connnection tady Package/StorageBackend/src/LegacyBackendConfig/SnowflakeConfig.php ?

jirkasemmler · 2023-09-22T06:30:38Z

packages/php-db-import-export/src/Backend/Snowflake/SnowflakeImportOptions.php

        bool $nullManipulation = self::NULL_MANIPULATION_ENABLED,
-        array $ignoreColumns = []
+        array $ignoreColumns = [],
+        array $autoCastTypes = [],


plus mohla by ta option dostat kus komentare? co to dela? to je strasnej pain tady tech options, ze pak clovek musi rozmotavat kde se to pouziva a jake to ma dopady

jirkasemmler · 2023-09-22T06:31:36Z

packages/php-db-import-export/src/Backend/Snowflake/SnowflakeImportOptions.php

        bool $nullManipulation = self::NULL_MANIPULATION_ENABLED,
-        array $ignoreColumns = []
+        array $ignoreColumns = [],
+        array $autoCastTypes = [],


toto je to co bych tam chtel https://github.com/keboola/storage-backend/pull/90/files#r1331117689

jirkasemmler · 2023-09-22T07:59:16Z

packages/php-db-import-export/src/Backend/Snowflake/ToFinalTable/SqlBuilder.php

+                $useAutoCast = in_array($type, $importOptions->autoCastTypes(), true);
+                $isSameType = $type === $sourceColumn->getColumnDefinition()->getType();
+                if ($useAutoCast && !$isSameType) {
+                    if ($type === Snowflake::TYPE_OBJECT) {


tady ocekavame to ze pro dalsi edgecasy co prijdou, tak se to bude vetvit, ze? Nechces to nekam vycuknout? kdyz to nechame takto, tak s dalsim typem prijde copypaste stejneho ifu. Ale asi je to pre-mature... necham na tobe

Prakjticky sem teď přidal test pro GEOMETRY a GEOGRAPHY víc takových typů tam není takže to dost pravděpodobně potřeba vůbec nebude pro nic dalšího.

jirkasemmler · 2023-09-22T08:00:09Z

packages/php-db-import-export/src/Backend/Snowflake/ToFinalTable/SqlBuilder.php

+                            $destinationColumn->getColumnDefinition()->getSQLDefinition(),
+                            SnowflakeQuote::quoteSingleIdentifier($destinationColumn->getColumnName())
+                        );
+                        continue;


ta metoda je takto nadesignovana, ale ten ty continue misto trochu lepe organizovaneho if else mi prijdou strasne neprehledne

if else by byl hrozný hell, možeš to zkusit jak by to vypadalo :) Možná match(true) by to trochu pošéfoval, ale takto jdeš po těch podmínkách a máš early return přes continue. Když je to "flat" přes else if tak bude strašně komplikované ty elseif poskládat správně pod sebe.

jirkasemmler · 2023-09-22T08:12:28Z

packages/php-db-import-export/tests/functional/Snowflake/SnowflakeBaseTestCase.php

-            false,
-            $useTimeStamp,
-            $skipLines
+            convertEmptyValuesToNull: [],


jirkasemmler · 2023-09-22T08:16:55Z

packages/php-db-import-export/tests/functional/Snowflake/ToFinal/FullImportTest.php

+    /**
+     * Test is testing loading of semi-structured data into typed table.
+     *
+     * We ignore here GEOGRAPHY and GEOMETRY as they act differently when casting from string


we ignore in this test... ale takze co se stane, kdyz to nekdo pouzije?

Hehe upravil sem sem on ten GEOMETRY a GEOGRAPHY funguje automaticky :) Takže sem ho přidal do testu.

zajca · 2023-09-22T09:47:25Z

Přihodil sem ještě error handling

SNFLK autocast types when using string stage table

0fa8ddb

zajca marked this pull request as draft September 19, 2023 13:45

zajca added 5 commits September 19, 2023 20:14

ignore _timestamp column in tests

4c4efe0

add SourceDestinationColumnMapTest

099a759

fix test cases

66953f2

add functional tests for sql builder

e69b473

drop isSameType

0c0390f

zajca commented Sep 20, 2023

View reviewed changes

zajca added 2 commits September 20, 2023 10:01

fix use-case

0cb3cb5

fix ignore multiple columns

3a61db2

zajca marked this pull request as ready for review September 20, 2023 10:04

zajca requested review from a team and jirkasemmler and removed request for a team September 20, 2023 10:04

zajca changed the title ~~CT-1116 SNFLK autocast types when using string stage table~~ CT-1128 SNFLK autocast types when using string stage table Sep 21, 2023

jirkasemmler requested changes Sep 22, 2023

View reviewed changes

remove autoCastTypes option, update test add GEOGRAPHY, GEOMETRY type

46b5ffd

zajca requested a review from jirkasemmler September 22, 2023 08:55

add tests for casting exceptions

d534718

jirkasemmler approved these changes Sep 22, 2023

View reviewed changes

zajca merged commit 8d80886 into main Sep 25, 2023

zajca deleted the zajca-ct-1116 branch September 25, 2023 06:02

Comments

Conversation

zajca commented Sep 19, 2023 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zajca Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zajca Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jirkasemmler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zajca commented Sep 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zajca commented Sep 19, 2023 •

edited by atlassian bot

Loading

zajca Sep 22, 2023 •

edited

Loading

zajca Sep 20, 2023 •

edited

Loading