Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add check and test for non-scalar values in primary key #8

Merged
merged 1 commit into from
Aug 31, 2018

Conversation

ujovlado
Copy link
Member

Fixes #7

@ujovlado ujovlado force-pushed the ujovlado-disallow-non-scalar-pk-values branch 3 times, most recently from ac548f7 to a525dce Compare August 30, 2018 15:15
@ujovlado ujovlado force-pushed the ujovlado-disallow-non-scalar-pk-values branch from a525dce to d456667 Compare August 30, 2018 15:27
@ujovlado ujovlado requested a review from MiroCillik August 30, 2018 15:31
@codecov-io
Copy link

codecov-io commented Aug 30, 2018

Codecov Report

Merging #8 into master will increase coverage by 0.18%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master       #8      +/-   ##
============================================
+ Coverage     95.65%   95.83%   +0.18%     
- Complexity       70       74       +4     
============================================
  Files             2        2              
  Lines           138      144       +6     
============================================
+ Hits            132      138       +6     
  Misses            6        6
Impacted Files Coverage Δ Complexity Δ
src/Keboola/CsvMap/Mapper.php 97.12% <100%> (+0.12%) 72 <4> (+4) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 857aa88...d456667. Read the comment docs.

Copy link
Member

@MiroCillik MiroCillik left a comment

Choose a reason for hiding this comment

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

"$oid": "5716054bee6e764c94fa85a6"

Takto sa zapisuje nejaky object v JSONe? Ci to je nejaka Mongo vec?

A "problem" je teda v tom mappingu 'coord' typu table? Ze v tom poli mozu byt len stringy a nie objekty?
Vyzera to dobre, len sa to snazim trochu pochopit od pohladu :)

@ujovlado
Copy link
Member Author

Je to standardny zapis, akurat je v key dolar. mam tento JSON:

{
    "_id": {
      "\$oid": "5716054bee6e764c94fa85a6"
    },
    "coord": [
      {
        "a": 1
      },
      {
        "a": 2
      }
    ]
  }

Problem nastane, ked si nastavim mapping len cez _id, vtedy sa zobere cela ta cast:

{
      "\$oid": "5716054bee6e764c94fa85a6"
    }

a nasledne sa nastavi ako PK.

Ked na to zavolam json_decode, dostanem:

object(stdClass)#1 (1) {
  ["$oid"]=>
  string(24) "5716054bee6e764c94fa85a6"
}
// resp. keby bol assoc param na true:
array(1) {
  ["$oid"]=>
  string(24) "5716054bee6e764c94fa85a6"
}

Co nie je validna hodnota pre primarny kluc.

Spravne musi byt ten mapping nastaveny _id.$oid a ked to nasledne nastavim ako PK, tak sa tam uz dostane len ten string.

@ujovlado
Copy link
Member Author

Zjednodusene povedane, toto je nie validny PK (teraz neriesim ci je to stdClass alebo array):

[
  ["$oid" => "5716054bee6e764c94fa85a6"]
]

@ujovlado
Copy link
Member Author

ujovlado commented Aug 31, 2018

Tento mapping je zly:

$config = [
            '_id' => [
                'type' => 'column',
                'mapping' => [
                    'destination' => 'id',
                    'primaryKey' => true,
                ]
            ],
            'coord' => [
                'type' => 'table',
                'destination' => 'coord',
                'tableMapping' => [
                    'a' => 'a',
                ]
            ]
        ];

Tento uz je ok:

$config = [
            '_id.$oid' => [
                'type' => 'column',
                'mapping' => [
                    'destination' => 'id',
                    'primaryKey' => true,
                ]
            ],
            'coord' => [
                'type' => 'table',
                'destination' => 'coord',
                'tableMapping' => [
                    'a' => 'a',
                ]
            ]
        ];

@MiroCillik
Copy link
Member

Jasne, uz rozumiem.

@MiroCillik
Copy link
Member

A null ako PK je teda v pohode? Inak za mna OK.

@ujovlado
Copy link
Member Author

No null je tiez v podstate scalar ... nechal som to tam preto, lebo s tym pocita niektory z testov (nechcel som to rozbit).

@ujovlado ujovlado merged commit 61871b4 into master Aug 31, 2018
@ujovlado ujovlado deleted the ujovlado-disallow-non-scalar-pk-values branch August 31, 2018 09:52
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants