keboola / php-db-import

MIT License
4 stars 0 forks source link

optional timestamp #11

Closed pivnicek closed 8 years ago

pivnicek commented 8 years ago

So, was thinking something like this to make _timestamps optional. If you agree, I'll write up the tests and RS version etc...

Halama commented 8 years ago

@pivnicek it look good but some array $options parameter whould be better than boolean flag parameter. So then it code it would look like:

$import->import('my-table', $columns, $sourceData, [
  'addTimestamp' => false,
]);
Halama commented 8 years ago

Also it would be more clear to put this as parameter of insertAllIntoTargetTable instead of make it class variable.

pivnicek commented 8 years ago

Great! Agreed. I'll flush it out.

pivnicek commented 8 years ago

I guess I'll add it for insertOrUpdateTargetTable also. If we'll support optional timestamp it should be completely supported imo.

Halama commented 8 years ago

@pivnicek ok, add it to insertOrUpdateTargetTable please too

Halama commented 8 years ago

don't forget to tests, every feature should be developed by tests.

pivnicek commented 8 years ago

OK, this may need a sanity check, but this should cover support for RS and SNFLK full table and incremental loading optional timestamp usage.

Halama commented 8 years ago

@pivnicek the tests are failing

pivnicek commented 8 years ago

damn, sorry :/ mmt

pivnicek commented 8 years ago

that's better