Conversation
apavlo
left a comment
apavlo
left a comment
There was a problem hiding this comment.
I've requested some minor fixes.
@dvanaken We should probably talk about some of the additional configuration parameters that we can move into DatabaseType. This DBMS doesn't support NULL so they use -1. It also requires the DB name to be prefixed in front of all column names.
| + TPCCUtil.randomStr(len - startORIGINAL - 9); | ||
| } | ||
|
|
||
| stock.s_dist_01 = TPCCUtil.randomStr(24); |
There was a problem hiding this comment.
Please rebase on the master branch to remove this regression.
| ordrPrepStmt.setInt(idx++, oorder.o_carrier_id); | ||
| } else { | ||
| ordrPrepStmt.setNull(idx++, Types.INTEGER); | ||
| ordrPrepStmt.setInt(idx++, -1); |
There was a problem hiding this comment.
Is this Clickhouse specific? What does "-1" mean?
There was a problem hiding this comment.
-1 is just an arbitrary choice. This is supposed to replace the null value but -1 might not be a good choice.
| OL_SUPPLY_W_ID int, | ||
| OL_QUANTITY Float32, | ||
| OL_DIST_INFO FixedString(24), | ||
| EventDate Date DEFAULT toDate(now()), |
There was a problem hiding this comment.
Please make the column names match the case of the other columns in the table. Also, add comments that explain what this does and why it is needed.
| */ | ||
| public static String getInsertSQL(Table catalog_tbl, boolean escape_names, int...exclude_columns) { | ||
| return getInsertSQL(catalog_tbl, false, false, 1, exclude_columns); | ||
| return getInsertSQL(catalog_tbl, true, false, 1, exclude_columns); |
There was a problem hiding this comment.
This is Clickhouse specific, right? What part of the code needed this?
There was a problem hiding this comment.
Yes this is so that the column names appear in the insert statements since we added two columns per table (we use the default values for those columns).
2 participants
No description provided.