[GitHub] incubator-trafodion pull request #1254: [TRAFODION-2719] Check for truncatio...

2017-10-03 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-trafodion/pull/1254


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread zellerh
Github user zellerh commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142546324
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
+  ValueId targetValId;
+  NABoolean found = FALSE;
+  for (CollIndex ni = 0; (!found) && (ni < 
targetRecExprArray.entries()); ni++)
+{
+  const ItemExpr *assignExpr = 
targetRecExprArray[ni].getItemExpr();
+  targetValId = 
assignExpr->child(0)->castToItemExpr()->getValueId();
+  NAColumn *targetCol = NULL; 
+  if (targetValId.getItemExpr()->getOperatorType() == 
ITM_BASECOLUMN)
+targetCol = 
((BaseColumn*)targetValId.getItemExpr())->getNAColumn();
+  else if (targetValId.getItemExpr()->getOperatorType() == 
ITM_INDEXCOLUMN)
+targetCol = 
((IndexColumn*)targetValId.getItemExpr())->getNAColumn();
+
+  if ((targetCol) &&
--- End diff --

Yes, sorry to comment on surround code. If both source and target share the 
same NATable, then wouldn't the following be sufficient:

```c++
  if ((targetCol) && courceCol == targetCol)
...
```

Another option would be to compare their column number:

```c++
   CMPASSERT(sourceCol->getNATable() == targetCol->getNATable());
   if (targetCol && sourceCol->getPosition() == targetCol->getPosition())
  ...
```

One thing might be a problem: I wonder whether in some cases we might 
suppress caching of the NATable on the insert side and allow it on the select 
side or vice versa.

I wonder whether @sureshsubbiah has any additional comments on this topic.


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread zellerh
Github user zellerh commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142544778
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
+  ValueId targetValId;
+  NABoolean found = FALSE;
+  for (CollIndex ni = 0; (!found) && (ni < 
targetRecExprArray.entries()); ni++)
+{
+  const ItemExpr *assignExpr = 
targetRecExprArray[ni].getItemExpr();
+  targetValId = 
assignExpr->child(0)->castToItemExpr()->getValueId();
+  NAColumn *targetCol = NULL; 
+  if (targetValId.getItemExpr()->getOperatorType() == 
ITM_BASECOLUMN)
+targetCol = 
((BaseColumn*)targetValId.getItemExpr())->getNAColumn();
+  else if (targetValId.getItemExpr()->getOperatorType() == 
ITM_INDEXCOLUMN)
+targetCol = 
((IndexColumn*)targetValId.getItemExpr())->getNAColumn();
+
+  if ((targetCol) &&
+  (targetCol->getColName() == sourceCol->getColName()) &&
+  (targetCol->getHbaseColFam() == sourceCol->getHbaseColFam()) 
&&
+  (targetCol->getHbaseColQual() == 
sourceCol->getHbaseColQual()) &&
+  
(targetCol->getNATable()->getTableName().getQualifiedNameAsAnsiString() ==
+   
sourceCol->getNATable()->getTableName().getQualifiedNameAsAnsiString()))
+{
+  found = TRUE;
+  break;
+}
+}
+
+  if (found)
+   {
+  Attributes * sourceValAttr = (generator->addMapInfo(sourceValId, 
0))->getAttr();
+  Attributes * targetValAttr = (generator->getMapInfo(targetValId, 
0))->getAttr();
+
+  // Save original location attributes so we can change them back 
after
+  // generating the update constraint expression
+
+  Attributes * savedValAttr = new(generator->wHeap()) Attributes();
--- End diff --

Thanks. Here is one example, but there are many others: 
FileScan::preCodeGen() in file GenPreCode.cpp:

```C++
ValueIdMap divExprMap;
ValueId computedBeg;

// If hashJoinBeg is :sysHV1 and the computed column
// expression is A/100, then the begin value for
// the computed column is :sysHV1/100. Do this
// rewrite by using a ValueIdMap
divExprMap.addMapEntry(underlyingCol, hashJoinBeg);

divExprMap.rewriteValueIdDown(computedColExpr->getValueId(),
  computedBeg);
newBeg = computedBeg.getItemExpr();
```

There are also examples in file OptPhysRelExpr where we rewrite expressions 
on the select side of an insert/select into expressions on the insert side, 
using the ValueIdMap that can be accessed with Join::updateToSelectMap().


---


[GitHub] incubator-trafodion pull request #1250: [TRAFODION-2758] Sort operator that ...

2017-10-03 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-trafodion/pull/1250


---


[GitHub] incubator-trafodion pull request #1254: [TRAFODION-2719] Check for truncatio...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1254#discussion_r142542077
  
--- Diff: core/sql/optimizer/OptLogRelExpr.cpp ---
@@ -2394,7 +2394,7 @@ Join::synthConstraints(NormWA * normWAPtr)
   (void) leftGA.hasCardConstraint(minLeft,maxLeft);
   (void) rightGA.hasCardConstraint(minRight,maxRight);
 
-  if (maxLeft == 1)
+  if (maxLeft == 1 || isIndexJoin_)
--- End diff --

Thanks. I guess I was thinking about joins to the index from elsewhere. But 
that's not what this code is dealing with.


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142540125
  
--- Diff: core/sql/optimizer/BindRelExpr.cpp ---
@@ -11552,35 +11552,7 @@ RelExpr *Update::bindNode(BindWA *bindWA)
   NABoolean transformUpdateKey = 
updatesClusteringKeyOrUniqueIndexKey(bindWA);
   if (bindWA->errStatus()) // error occurred in updatesCKOrUniqueIndexKey()
 return this;
-  // To be removed when TRAFODION-1610 is implemented.
-  NABoolean xnsfrmHbaseUpdate = FALSE;
-  if ((hbaseOper()) && (NOT isMerge()))
-{  
-  if (CmpCommon::getDefault(HBASE_TRANSFORM_UPDATE_TO_DELETE_INSERT) 
== DF_ON)
--- End diff --

You are correct! Good catch. I will remove it.


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142539746
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
+  ValueId targetValId;
+  NABoolean found = FALSE;
+  for (CollIndex ni = 0; (!found) && (ni < 
targetRecExprArray.entries()); ni++)
+{
+  const ItemExpr *assignExpr = 
targetRecExprArray[ni].getItemExpr();
+  targetValId = 
assignExpr->child(0)->castToItemExpr()->getValueId();
+  NAColumn *targetCol = NULL; 
+  if (targetValId.getItemExpr()->getOperatorType() == 
ITM_BASECOLUMN)
+targetCol = 
((BaseColumn*)targetValId.getItemExpr())->getNAColumn();
+  else if (targetValId.getItemExpr()->getOperatorType() == 
ITM_INDEXCOLUMN)
+targetCol = 
((IndexColumn*)targetValId.getItemExpr())->getNAColumn();
+
+  if ((targetCol) &&
+  (targetCol->getColName() == sourceCol->getColName()) &&
+  (targetCol->getHbaseColFam() == sourceCol->getHbaseColFam()) 
&&
+  (targetCol->getHbaseColQual() == 
sourceCol->getHbaseColQual()) &&
+  
(targetCol->getNATable()->getTableName().getQualifiedNameAsAnsiString() ==
+   
sourceCol->getNATable()->getTableName().getQualifiedNameAsAnsiString()))
+{
+  found = TRUE;
+  break;
+}
+}
+
+  if (found)
+   {
+  Attributes * sourceValAttr = (generator->addMapInfo(sourceValId, 
0))->getAttr();
+  Attributes * targetValAttr = (generator->getMapInfo(targetValId, 
0))->getAttr();
+
+  // Save original location attributes so we can change them back 
after
+  // generating the update constraint expression
+
+  Attributes * savedValAttr = new(generator->wHeap()) Attributes();
+  savedValAttr->copyLocationAttrs(sourceValAttr);
+  savedSourceAttrsList.insert(savedValAttr);
+  savedSourceVIDlist.insert(sourceValId);
+
+  sourceValAttr->copyLocationAttrs(targetValAttr);
+}
+
+}
+
+  // Now that we have remapped the Attributes for the columns to their 
values
+  // in the new record, we can generate the update constraint expression.
+
+  expGen->generateExpr(constrTree->getValueId(), ex_expr::exp_SCAN_PRED,
--- End diff --

Good question. I will investigate.


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142539459
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
+  ValueId targetValId;
+  NABoolean found = FALSE;
+  for (CollIndex ni = 0; (!found) && (ni < 
targetRecExprArray.entries()); ni++)
+{
+  const ItemExpr *assignExpr = 
targetRecExprArray[ni].getItemExpr();
+  targetValId = 
assignExpr->child(0)->castToItemExpr()->getValueId();
+  NAColumn *targetCol = NULL; 
+  if (targetValId.getItemExpr()->getOperatorType() == 
ITM_BASECOLUMN)
+targetCol = 
((BaseColumn*)targetValId.getItemExpr())->getNAColumn();
+  else if (targetValId.getItemExpr()->getOperatorType() == 
ITM_INDEXCOLUMN)
+targetCol = 
((IndexColumn*)targetValId.getItemExpr())->getNAColumn();
+
+  if ((targetCol) &&
--- End diff --

I used the same technique that the existing code used. I'm curious how 
source and target tables could differ... check constraints are always local to 
a table, correct? That said, I'm happy to consider alternatives. Can you point 
me to an example?


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142539579
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
+  ValueId targetValId;
+  NABoolean found = FALSE;
+  for (CollIndex ni = 0; (!found) && (ni < 
targetRecExprArray.entries()); ni++)
+{
+  const ItemExpr *assignExpr = 
targetRecExprArray[ni].getItemExpr();
+  targetValId = 
assignExpr->child(0)->castToItemExpr()->getValueId();
+  NAColumn *targetCol = NULL; 
+  if (targetValId.getItemExpr()->getOperatorType() == 
ITM_BASECOLUMN)
+targetCol = 
((BaseColumn*)targetValId.getItemExpr())->getNAColumn();
+  else if (targetValId.getItemExpr()->getOperatorType() == 
ITM_INDEXCOLUMN)
+targetCol = 
((IndexColumn*)targetValId.getItemExpr())->getNAColumn();
+
+  if ((targetCol) &&
+  (targetCol->getColName() == sourceCol->getColName()) &&
+  (targetCol->getHbaseColFam() == sourceCol->getHbaseColFam()) 
&&
+  (targetCol->getHbaseColQual() == 
sourceCol->getHbaseColQual()) &&
+  
(targetCol->getNATable()->getTableName().getQualifiedNameAsAnsiString() ==
+   
sourceCol->getNATable()->getTableName().getQualifiedNameAsAnsiString()))
+{
+  found = TRUE;
+  break;
+}
+}
+
+  if (found)
+   {
+  Attributes * sourceValAttr = (generator->addMapInfo(sourceValId, 
0))->getAttr();
+  Attributes * targetValAttr = (generator->getMapInfo(targetValId, 
0))->getAttr();
+
+  // Save original location attributes so we can change them back 
after
+  // generating the update constraint expression
+
+  Attributes * savedValAttr = new(generator->wHeap()) Attributes();
--- End diff --

Happy to consider. Can you point me to an instructive example?


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142539058
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
--- End diff --

Thanks. Will add.


---


[GitHub] incubator-trafodion pull request #1247: [TRAFODION-2755] Create *Trafodion S...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1247#discussion_r142537859
  
--- Diff: docs/lob_guide/src/asciidoc/_chapters/working_with_lob.adoc ---
@@ -751,4 +751,380 @@ CQD `LOB_MAX_SIZE` (default 10G expressed in M 
[1M]).
 
 * Extract Target Locations
 +
-The file to extract to can be a local linux file or a local HDFS file.
\ No newline at end of file
+The file to extract to can be a local linux file or a local HDFS file.
+
+[#deleting column from a sql table containing lob columns]
+== Deleting Column from a SQL Table Containing LOB columns
+
+[#syntax]
+=== Syntax
+
+```
+DELETE lob-column-name FROM table-name [WHERE CLAUSE]
+```
+
+[#considerations]
+=== Considerations
+
+When one or more rows containing LOB columns are deleted from LOB table, 
only the metadata information is dropped and the hdfs data remains as it is. 
The references to the lob data are removed from the lob descriptor file. 
+
+This mechanism has not been implemented yet as a separate utility but it 
is triggered as a part of insert, update and append operations. For more 
information, see <>.
+
+[#dropping a sql table containing lob columns ]
+== Dropping a SQL Table Containing LOB Columns 
+
+Drop works like any other drop table. All dependent tables are deleted. 
All files in hdfs (data and descriptor) files are also deleted.
+
+For more information, see <> in 
http://trafodion.incubator.apache.org/docs/sql_reference/index.html[Trafodion 
SQL Reference Manual].
+
+[#garbage collection]
+== Garbage Collection
+
+When a lob datafile for a column has reached a certain limit, defined by a 
CQD `LOB_GC_LIMIT_SIZE`, then a compaction is triggered automatically. +
+The default GC Limit is 10GB and can be changed if needed. 
+
+The need for GC arises because when a delete operation or an update 
operation is performed, the old data black in the hdfs file will be left as 
unused. +
+In the case of update, the old data will be left as unused and the new 
data will be written into a new section, so all these “holes” in the LOB 
data file are needlessly occupying space. 
--- End diff --

Consider: "... and the new data will be written into a new section, leaving 
"holes" in the LOB data file that needlessly take up space."


---


[GitHub] incubator-trafodion pull request #1247: [TRAFODION-2755] Create *Trafodion S...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1247#discussion_r142538817
  
--- Diff: docs/lob_guide/src/asciidoc/_chapters/working_with_lob.adoc ---
@@ -751,4 +751,380 @@ CQD `LOB_MAX_SIZE` (default 10G expressed in M 
[1M]).
 
 * Extract Target Locations
 +
-The file to extract to can be a local linux file or a local HDFS file.
\ No newline at end of file
+The file to extract to can be a local linux file or a local HDFS file.
+
+[#deleting column from a sql table containing lob columns]
+== Deleting Column from a SQL Table Containing LOB columns
+
+[#syntax]
+=== Syntax
+
+```
+DELETE lob-column-name FROM table-name [WHERE CLAUSE]
+```
+
+[#considerations]
+=== Considerations
+
+When one or more rows containing LOB columns are deleted from LOB table, 
only the metadata information is dropped and the hdfs data remains as it is. 
The references to the lob data are removed from the lob descriptor file. 
+
+This mechanism has not been implemented yet as a separate utility but it 
is triggered as a part of insert, update and append operations. For more 
information, see <>.
+
+[#dropping a sql table containing lob columns ]
+== Dropping a SQL Table Containing LOB Columns 
+
+Drop works like any other drop table. All dependent tables are deleted. 
All files in hdfs (data and descriptor) files are also deleted.
+
+For more information, see <> in 
http://trafodion.incubator.apache.org/docs/sql_reference/index.html[Trafodion 
SQL Reference Manual].
+
+[#garbage collection]
+== Garbage Collection
+
+When a lob datafile for a column has reached a certain limit, defined by a 
CQD `LOB_GC_LIMIT_SIZE`, then a compaction is triggered automatically. +
+The default GC Limit is 10GB and can be changed if needed. 
+
+The need for GC arises because when a delete operation or an update 
operation is performed, the old data black in the hdfs file will be left as 
unused. +
+In the case of update, the old data will be left as unused and the new 
data will be written into a new section, so all these “holes” in the LOB 
data file are needlessly occupying space. 
+
+The LOB descriptor chunks file is looked at to see which ranges and 
offsets are actually used. The LOB datafile is temporarily saved. The 
compaction is done into a new tempfile. When the sections have all been copied 
into the tempfile, Trafodion will delete the existing lob data file and rename 
the tempfile. 
+
+Finally, the saved copy of the LOB datafile is dropped. The saved copy is 
there just in case you need to fall back to it in case of an error. Since this 
operation is triggered as part of an IUD operation, a definite slowdown will 
occur for that insert/update operation compared to subsequent inserts/updates. 
+
+Also, each lob column of a table can be compacted separately as needed. GC 
does not have to be done to all columns of the LOB table all at once. 
+
+NOTE: Currently the GC is done in the same transaction as the transaction 
being used for the insert or update operation. If any part of the GC fails, 
then the entire transaction is aborted. 
+
+When Trafodion has support for local transactions, Trafodion will do the 
GC in a separate transaction or in a separate process, so you can fail the GC 
with a warning and allow the insert to go through. 
+
+Setting the CQD `LOB_GC_LIMIT_SIZE` to 0 would prevent GC from occurring.
+
+[#cleanup of a sql table containing lob columns]
+== Cleanup of a SQL Table Containing LOB Columns
+
+Cleanup works like cleanup of any other table. The command ensures all 
dependent SQL LOB tables and hdfs files are dropped ignoring errors if any.
+
+For more information, see <> in 
http://trafodion.incubator.apache.org/docs/sql_reference/index.html[Trafodion 
SQL Reference Manual].
+
+[#showddl for lob]
+== SHOWDDL for LOB
+
+SHOWDDL for LOB with a special option will show all the dependent objects, 
names and details about the table.
+
+[#syntax]
+=== Syntax
+
+```
+SHOWDDL table-name, LOB DETAILS
+```
+
+[#examples]
+=== Examples
+
+* This example displays the details of the table t1ob1.
+
++
+
+```
+>>SHOWDDL tlob1, LOB DETAILS;
+CREATE TABLE TRAFODION.SEABASE.TLOB1
+  (
+C1   INT NO DEFAULT NOT NULL NOT DROPPABLE 
SERIALIZED
+  , C2   BLOB DEFAULT NULL NOT SERIALIZED
+  , PRIMARY KEY (C1 ASC)
+  )
+;
+
+LOB Metadata
+
+
+CREATE TABLE TRAFODION.SEABASE.LOBMD_04239091936503896833
+  (
+LOBNUM  

[GitHub] incubator-trafodion pull request #1247: [TRAFODION-2755] Create *Trafodion S...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1247#discussion_r142538517
  
--- Diff: docs/lob_guide/src/asciidoc/_chapters/working_with_lob.adoc ---
@@ -751,4 +751,380 @@ CQD `LOB_MAX_SIZE` (default 10G expressed in M 
[1M]).
 
 * Extract Target Locations
 +
-The file to extract to can be a local linux file or a local HDFS file.
\ No newline at end of file
+The file to extract to can be a local linux file or a local HDFS file.
+
+[#deleting column from a sql table containing lob columns]
+== Deleting Column from a SQL Table Containing LOB columns
+
+[#syntax]
+=== Syntax
+
+```
+DELETE lob-column-name FROM table-name [WHERE CLAUSE]
+```
+
+[#considerations]
+=== Considerations
+
+When one or more rows containing LOB columns are deleted from LOB table, 
only the metadata information is dropped and the hdfs data remains as it is. 
The references to the lob data are removed from the lob descriptor file. 
+
+This mechanism has not been implemented yet as a separate utility but it 
is triggered as a part of insert, update and append operations. For more 
information, see <>.
+
+[#dropping a sql table containing lob columns ]
+== Dropping a SQL Table Containing LOB Columns 
+
+Drop works like any other drop table. All dependent tables are deleted. 
All files in hdfs (data and descriptor) files are also deleted.
+
+For more information, see <> in 
http://trafodion.incubator.apache.org/docs/sql_reference/index.html[Trafodion 
SQL Reference Manual].
+
+[#garbage collection]
+== Garbage Collection
+
+When a lob datafile for a column has reached a certain limit, defined by a 
CQD `LOB_GC_LIMIT_SIZE`, then a compaction is triggered automatically. +
+The default GC Limit is 10GB and can be changed if needed. 
+
+The need for GC arises because when a delete operation or an update 
operation is performed, the old data black in the hdfs file will be left as 
unused. +
+In the case of update, the old data will be left as unused and the new 
data will be written into a new section, so all these “holes” in the LOB 
data file are needlessly occupying space. 
+
+The LOB descriptor chunks file is looked at to see which ranges and 
offsets are actually used. The LOB datafile is temporarily saved. The 
compaction is done into a new tempfile. When the sections have all been copied 
into the tempfile, Trafodion will delete the existing lob data file and rename 
the tempfile. 
+
+Finally, the saved copy of the LOB datafile is dropped. The saved copy is 
there just in case you need to fall back to it in case of an error. Since this 
operation is triggered as part of an IUD operation, a definite slowdown will 
occur for that insert/update operation compared to subsequent inserts/updates. 
+
+Also, each lob column of a table can be compacted separately as needed. GC 
does not have to be done to all columns of the LOB table all at once. 
+
+NOTE: Currently the GC is done in the same transaction as the transaction 
being used for the insert or update operation. If any part of the GC fails, 
then the entire transaction is aborted. 
+
+When Trafodion has support for local transactions, Trafodion will do the 
GC in a separate transaction or in a separate process, so you can fail the GC 
with a warning and allow the insert to go through. 
--- End diff --

Consider: "When Trafodion has support for local transactions, Trafodion 
will do the GC in a separate transaction or in a separate process, so GC can 
fail with a warning yet the insert can continue." (I'm assuming it is the 
system that causes the GC to fail and not the end-user. If I'm wrong, ignore 
this comment.)


---


[GitHub] incubator-trafodion pull request #1247: [TRAFODION-2755] Create *Trafodion S...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1247#discussion_r142537556
  
--- Diff: docs/lob_guide/src/asciidoc/_chapters/working_with_lob.adoc ---
@@ -751,4 +751,380 @@ CQD `LOB_MAX_SIZE` (default 10G expressed in M 
[1M]).
 
 * Extract Target Locations
 +
-The file to extract to can be a local linux file or a local HDFS file.
\ No newline at end of file
+The file to extract to can be a local linux file or a local HDFS file.
+
+[#deleting column from a sql table containing lob columns]
+== Deleting Column from a SQL Table Containing LOB columns
+
+[#syntax]
+=== Syntax
+
+```
+DELETE lob-column-name FROM table-name [WHERE CLAUSE]
+```
+
+[#considerations]
+=== Considerations
+
+When one or more rows containing LOB columns are deleted from LOB table, 
only the metadata information is dropped and the hdfs data remains as it is. 
The references to the lob data are removed from the lob descriptor file. 
+
+This mechanism has not been implemented yet as a separate utility but it 
is triggered as a part of insert, update and append operations. For more 
information, see <>.
+
+[#dropping a sql table containing lob columns ]
+== Dropping a SQL Table Containing LOB Columns 
+
+Drop works like any other drop table. All dependent tables are deleted. 
All files in hdfs (data and descriptor) files are also deleted.
+
+For more information, see <> in 
http://trafodion.incubator.apache.org/docs/sql_reference/index.html[Trafodion 
SQL Reference Manual].
+
+[#garbage collection]
+== Garbage Collection
+
+When a lob datafile for a column has reached a certain limit, defined by a 
CQD `LOB_GC_LIMIT_SIZE`, then a compaction is triggered automatically. +
+The default GC Limit is 10GB and can be changed if needed. 
--- End diff --

Should define the GC acronym. For example, "The default Garbage Collection 
(GC) limit is ..." Then you can use GC later when you mean Garbage Collection.


---


[GitHub] incubator-trafodion pull request #1247: [TRAFODION-2755] Create *Trafodion S...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1247#discussion_r142538725
  
--- Diff: docs/lob_guide/src/asciidoc/_chapters/working_with_lob.adoc ---
@@ -751,4 +751,380 @@ CQD `LOB_MAX_SIZE` (default 10G expressed in M 
[1M]).
 
 * Extract Target Locations
 +
-The file to extract to can be a local linux file or a local HDFS file.
\ No newline at end of file
+The file to extract to can be a local linux file or a local HDFS file.
+
+[#deleting column from a sql table containing lob columns]
+== Deleting Column from a SQL Table Containing LOB columns
+
+[#syntax]
+=== Syntax
+
+```
+DELETE lob-column-name FROM table-name [WHERE CLAUSE]
+```
+
+[#considerations]
+=== Considerations
+
+When one or more rows containing LOB columns are deleted from LOB table, 
only the metadata information is dropped and the hdfs data remains as it is. 
The references to the lob data are removed from the lob descriptor file. 
+
+This mechanism has not been implemented yet as a separate utility but it 
is triggered as a part of insert, update and append operations. For more 
information, see <>.
+
+[#dropping a sql table containing lob columns ]
+== Dropping a SQL Table Containing LOB Columns 
+
+Drop works like any other drop table. All dependent tables are deleted. 
All files in hdfs (data and descriptor) files are also deleted.
+
+For more information, see <> in 
http://trafodion.incubator.apache.org/docs/sql_reference/index.html[Trafodion 
SQL Reference Manual].
+
+[#garbage collection]
+== Garbage Collection
+
+When a lob datafile for a column has reached a certain limit, defined by a 
CQD `LOB_GC_LIMIT_SIZE`, then a compaction is triggered automatically. +
+The default GC Limit is 10GB and can be changed if needed. 
+
+The need for GC arises because when a delete operation or an update 
operation is performed, the old data black in the hdfs file will be left as 
unused. +
+In the case of update, the old data will be left as unused and the new 
data will be written into a new section, so all these “holes” in the LOB 
data file are needlessly occupying space. 
+
+The LOB descriptor chunks file is looked at to see which ranges and 
offsets are actually used. The LOB datafile is temporarily saved. The 
compaction is done into a new tempfile. When the sections have all been copied 
into the tempfile, Trafodion will delete the existing lob data file and rename 
the tempfile. 
+
+Finally, the saved copy of the LOB datafile is dropped. The saved copy is 
there just in case you need to fall back to it in case of an error. Since this 
operation is triggered as part of an IUD operation, a definite slowdown will 
occur for that insert/update operation compared to subsequent inserts/updates. 
+
+Also, each lob column of a table can be compacted separately as needed. GC 
does not have to be done to all columns of the LOB table all at once. 
+
+NOTE: Currently the GC is done in the same transaction as the transaction 
being used for the insert or update operation. If any part of the GC fails, 
then the entire transaction is aborted. 
+
+When Trafodion has support for local transactions, Trafodion will do the 
GC in a separate transaction or in a separate process, so you can fail the GC 
with a warning and allow the insert to go through. 
+
+Setting the CQD `LOB_GC_LIMIT_SIZE` to 0 would prevent GC from occurring.
+
+[#cleanup of a sql table containing lob columns]
+== Cleanup of a SQL Table Containing LOB Columns
+
+Cleanup works like cleanup of any other table. The command ensures all 
dependent SQL LOB tables and hdfs files are dropped ignoring errors if any.
+
+For more information, see <> in 
http://trafodion.incubator.apache.org/docs/sql_reference/index.html[Trafodion 
SQL Reference Manual].
+
+[#showddl for lob]
+== SHOWDDL for LOB
+
+SHOWDDL for LOB with a special option will show all the dependent objects, 
names and details about the table.
+
+[#syntax]
+=== Syntax
+
+```
+SHOWDDL table-name, LOB DETAILS
+```
+
+[#examples]
+=== Examples
+
+* This example displays the details of the table t1ob1.
+
++
+
+```
+>>SHOWDDL tlob1, LOB DETAILS;
+CREATE TABLE TRAFODION.SEABASE.TLOB1
+  (
+C1   INT NO DEFAULT NOT NULL NOT DROPPABLE 
SERIALIZED
+  , C2   BLOB DEFAULT NULL NOT SERIALIZED
+  , PRIMARY KEY (C1 ASC)
+  )
+;
+
+LOB Metadata
+
+
+CREATE TABLE TRAFODION.SEABASE.LOBMD_04239091936503896833
+  (
+LOBNUM  

[GitHub] incubator-trafodion pull request #1254: [TRAFODION-2719] Check for truncatio...

2017-10-03 Thread zellerh
Github user zellerh commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1254#discussion_r142535910
  
--- Diff: core/sql/optimizer/OptLogRelExpr.cpp ---
@@ -2394,7 +2394,7 @@ Join::synthConstraints(NormWA * normWAPtr)
   (void) leftGA.hasCardConstraint(minLeft,maxLeft);
   (void) rightGA.hasCardConstraint(minRight,maxRight);
 
-  if (maxLeft == 1)
+  if (maxLeft == 1 || isIndexJoin_)
--- End diff --

The non-unique index contains one row for each base table row, and it 
contains the clustering key of that row. So, there is still a 1:1 match between 
the non-unique index and the base table/clustering index, and a probe cache 
would not have any cache hits.


---


[GitHub] incubator-trafodion pull request #1243: [TRAFODION-2744] Correct the Descrip...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1243#discussion_r142534940
  
--- Diff: docs/odb_user/src/asciidoc/_chapters/load.adoc ---
@@ -117,7 +117,7 @@ If you add a `+` sign in front of the file-name, odb  
*appends* to ``inste
  +
 - one thread to read from the input file and +
 - as many threads as the parallel argument to write via ODBC. This option 
is database independent.
-| `errmax=num` | odb prints up to num error messages per rowset. Normally 
used with soe to limit the number of error messages printed to the standard 
error
+| `errmax=num` | odb prints up to num error messages per rowset. Normally 
used with soe to limit the number of error messages printed to the standard 
error.
--- End diff --

Perhaps "... standard error stream."


---


[GitHub] incubator-trafodion pull request #1243: [TRAFODION-2744] Correct the Descrip...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1243#discussion_r142534687
  
--- Diff: docs/odb_user/src/asciidoc/_chapters/load.adoc ---
@@ -1080,24 +1080,54 @@ define the size of this buffer in two different 
ways: +
 | `ifempty` | Loads the target table only if empty.
 | `nomark` | Don’t print the number of records loaded so far during 
loads.
 | `soe` | Stop On Error. odb stops as soon as it encounters an error.
-| `parallel=num` | odb uses as many threads as the parallel argument to 
extract data from partitioned source tables
-*PLUS* an equivalent number of threads to write to the target table. +
- +
- *Example* +
-  +
-If you specify `parallel=4` and the source table is made of 32 partitions, 
then odb start *four* threads
-(four ODBC connections) to read from the source table *PLUS* four threads 
(four ODBC connections) to write to the target table: +
- +
-- thread 0 extracts partitions 0-7 from source +
-- thread 1 writes data extracted from thread 0 to target +
-- thread 2 extracts partitions 8-15 from source +
-- thread 3 writes data extracted from thread 2 to target +
-- thread 4 extracts partitions 16-23 from source +
-- thread 5 writes data extracted from thread 4 to target +
-- thread 6 extracts partitions 24-31 from source +
-- thread 7 writes data extracted from thread 6 to target +
- +
-*You have to specify `splitby`.*
+| `parallel=num` | odb uses two kinds of threads: +
+
+ To extract data from partitioned source table. +
+The number of the threads is as many as the parallel argument. +
+
+ To write data to the target table. +
+The number of the threads is equal to *parallel argument* * *number of 
loaders*. + 
   
+
+*Example* 
+
+If you specify parallel argument = 4 and the source table is made of 32 
partitions, then odb start: +
--- End diff --

Grammar: Consider "If you specify parallel argument = 4 and the source 
table has 32 partitions, then odb starts: +"


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread zellerh
Github user zellerh commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142513113
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
--- End diff --

An assert that the ItemExpr here is really an index column might be good.


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread zellerh
Github user zellerh commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142527381
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
+  ValueId targetValId;
+  NABoolean found = FALSE;
+  for (CollIndex ni = 0; (!found) && (ni < 
targetRecExprArray.entries()); ni++)
+{
+  const ItemExpr *assignExpr = 
targetRecExprArray[ni].getItemExpr();
+  targetValId = 
assignExpr->child(0)->castToItemExpr()->getValueId();
+  NAColumn *targetCol = NULL; 
+  if (targetValId.getItemExpr()->getOperatorType() == 
ITM_BASECOLUMN)
+targetCol = 
((BaseColumn*)targetValId.getItemExpr())->getNAColumn();
+  else if (targetValId.getItemExpr()->getOperatorType() == 
ITM_INDEXCOLUMN)
+targetCol = 
((IndexColumn*)targetValId.getItemExpr())->getNAColumn();
+
+  if ((targetCol) &&
--- End diff --

This code to find the corresponding column seems risky to me. Isn't there a 
way to find the column by position? If the NATables for source and target are 
the same, that should be possible. If they are not, then matching by name is 
probably not the right thing anyway.


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread zellerh
Github user zellerh commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142531876
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
+  ValueId targetValId;
+  NABoolean found = FALSE;
+  for (CollIndex ni = 0; (!found) && (ni < 
targetRecExprArray.entries()); ni++)
+{
+  const ItemExpr *assignExpr = 
targetRecExprArray[ni].getItemExpr();
+  targetValId = 
assignExpr->child(0)->castToItemExpr()->getValueId();
+  NAColumn *targetCol = NULL; 
+  if (targetValId.getItemExpr()->getOperatorType() == 
ITM_BASECOLUMN)
+targetCol = 
((BaseColumn*)targetValId.getItemExpr())->getNAColumn();
+  else if (targetValId.getItemExpr()->getOperatorType() == 
ITM_INDEXCOLUMN)
+targetCol = 
((IndexColumn*)targetValId.getItemExpr())->getNAColumn();
+
+  if ((targetCol) &&
+  (targetCol->getColName() == sourceCol->getColName()) &&
+  (targetCol->getHbaseColFam() == sourceCol->getHbaseColFam()) 
&&
+  (targetCol->getHbaseColQual() == 
sourceCol->getHbaseColQual()) &&
+  
(targetCol->getNATable()->getTableName().getQualifiedNameAsAnsiString() ==
+   
sourceCol->getNATable()->getTableName().getQualifiedNameAsAnsiString()))
+{
+  found = TRUE;
+  break;
+}
+}
+
+  if (found)
+   {
+  Attributes * sourceValAttr = (generator->addMapInfo(sourceValId, 
0))->getAttr();
+  Attributes * targetValAttr = (generator->getMapInfo(targetValId, 
0))->getAttr();
+
+  // Save original location attributes so we can change them back 
after
+  // generating the update constraint expression
+
+  Attributes * savedValAttr = new(generator->wHeap()) Attributes();
+  savedValAttr->copyLocationAttrs(sourceValAttr);
+  savedSourceAttrsList.insert(savedValAttr);
+  savedSourceVIDlist.insert(sourceValId);
+
+  sourceValAttr->copyLocationAttrs(targetValAttr);
+}
+
+}
+
+  // Now that we have remapped the Attributes for the columns to their 
values
+  // in the new record, we can generate the update constraint expression.
+
+  expGen->generateExpr(constrTree->getValueId(), ex_expr::exp_SCAN_PRED,
+   targetExpr);
+
+  // Now put the Attributes back the way they were.
+
+  for (Lng32 i = 0; i < savedSourceVIDlist.entries(); i++)
+{
+  ValueId sourceValId = savedSourceVIDlist[i];
+  Attributes * sourceValAttr = (generator->getMapInfo(sourceValId, 
0))->getAttr();
+  sourceValAttr->copyLocationAttrs(savedSourceAttrsList[i]);
--- End diff --

If we created a new map table entry on line 343, this code won't delete 
that entry. As far as I can tell, it would put back the temporary location that 
was assigned for a new entry? See comment above, I would recommend not 
modifying map tables. If you want to stay with the MapTable method, though, it 
might be better to drop those entries that were created on line 343 here.


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread zellerh
Github user zellerh commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142530211
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
+  ValueId targetValId;
+  NABoolean found = FALSE;
+  for (CollIndex ni = 0; (!found) && (ni < 
targetRecExprArray.entries()); ni++)
+{
+  const ItemExpr *assignExpr = 
targetRecExprArray[ni].getItemExpr();
+  targetValId = 
assignExpr->child(0)->castToItemExpr()->getValueId();
+  NAColumn *targetCol = NULL; 
+  if (targetValId.getItemExpr()->getOperatorType() == 
ITM_BASECOLUMN)
+targetCol = 
((BaseColumn*)targetValId.getItemExpr())->getNAColumn();
+  else if (targetValId.getItemExpr()->getOperatorType() == 
ITM_INDEXCOLUMN)
+targetCol = 
((IndexColumn*)targetValId.getItemExpr())->getNAColumn();
+
+  if ((targetCol) &&
+  (targetCol->getColName() == sourceCol->getColName()) &&
+  (targetCol->getHbaseColFam() == sourceCol->getHbaseColFam()) 
&&
+  (targetCol->getHbaseColQual() == 
sourceCol->getHbaseColQual()) &&
+  
(targetCol->getNATable()->getTableName().getQualifiedNameAsAnsiString() ==
+   
sourceCol->getNATable()->getTableName().getQualifiedNameAsAnsiString()))
+{
+  found = TRUE;
+  break;
+}
+}
+
+  if (found)
+   {
+  Attributes * sourceValAttr = (generator->addMapInfo(sourceValId, 
0))->getAttr();
+  Attributes * targetValAttr = (generator->getMapInfo(targetValId, 
0))->getAttr();
+
+  // Save original location attributes so we can change them back 
after
+  // generating the update constraint expression
+
+  Attributes * savedValAttr = new(generator->wHeap()) Attributes();
+  savedValAttr->copyLocationAttrs(sourceValAttr);
+  savedSourceAttrsList.insert(savedValAttr);
+  savedSourceVIDlist.insert(sourceValId);
+
+  sourceValAttr->copyLocationAttrs(targetValAttr);
+}
+
+}
+
+  // Now that we have remapped the Attributes for the columns to their 
values
+  // in the new record, we can generate the update constraint expression.
+
+  expGen->generateExpr(constrTree->getValueId(), ex_expr::exp_SCAN_PRED,
--- End diff --

I don't quite understand what happens when the constraint tree refers to 
columns that are not in the assign list, or is the assign list targetRecExpr 
always complete? If it's always complete, there would be an optimization to 
skip constraints that don't refer to any updated columns.


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread zellerh
Github user zellerh commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142534021
  
--- Diff: core/sql/optimizer/BindRelExpr.cpp ---
@@ -11552,35 +11552,7 @@ RelExpr *Update::bindNode(BindWA *bindWA)
   NABoolean transformUpdateKey = 
updatesClusteringKeyOrUniqueIndexKey(bindWA);
   if (bindWA->errStatus()) // error occurred in updatesCKOrUniqueIndexKey()
 return this;
-  // To be removed when TRAFODION-1610 is implemented.
-  NABoolean xnsfrmHbaseUpdate = FALSE;
-  if ((hbaseOper()) && (NOT isMerge()))
-{  
-  if (CmpCommon::getDefault(HBASE_TRANSFORM_UPDATE_TO_DELETE_INSERT) 
== DF_ON)
--- End diff --

I assume this is the only use of this CQD, so the CQD should be removed?


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread zellerh
Github user zellerh commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142529491
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
+  ValueId targetValId;
+  NABoolean found = FALSE;
+  for (CollIndex ni = 0; (!found) && (ni < 
targetRecExprArray.entries()); ni++)
+{
+  const ItemExpr *assignExpr = 
targetRecExprArray[ni].getItemExpr();
+  targetValId = 
assignExpr->child(0)->castToItemExpr()->getValueId();
+  NAColumn *targetCol = NULL; 
+  if (targetValId.getItemExpr()->getOperatorType() == 
ITM_BASECOLUMN)
+targetCol = 
((BaseColumn*)targetValId.getItemExpr())->getNAColumn();
+  else if (targetValId.getItemExpr()->getOperatorType() == 
ITM_INDEXCOLUMN)
+targetCol = 
((IndexColumn*)targetValId.getItemExpr())->getNAColumn();
+
+  if ((targetCol) &&
+  (targetCol->getColName() == sourceCol->getColName()) &&
+  (targetCol->getHbaseColFam() == sourceCol->getHbaseColFam()) 
&&
+  (targetCol->getHbaseColQual() == 
sourceCol->getHbaseColQual()) &&
+  
(targetCol->getNATable()->getTableName().getQualifiedNameAsAnsiString() ==
+   
sourceCol->getNATable()->getTableName().getQualifiedNameAsAnsiString()))
+{
+  found = TRUE;
+  break;
+}
+}
+
+  if (found)
+   {
+  Attributes * sourceValAttr = (generator->addMapInfo(sourceValId, 
0))->getAttr();
+  Attributes * targetValAttr = (generator->getMapInfo(targetValId, 
0))->getAttr();
+
+  // Save original location attributes so we can change them back 
after
+  // generating the update constraint expression
+
+  Attributes * savedValAttr = new(generator->wHeap()) Attributes();
--- End diff --

This is probably existing code that's modified, but I still have a comment: 
Instead of altering the map tables, it might be better to make a ValueIdMap m 
for the old and new values and calling m.rewriteValueIdUp(newValId, 
constrTree->getValueId()).


---


[GitHub] incubator-trafodion pull request #1254: [TRAFODION-2719] Check for truncatio...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1254#discussion_r142528667
  
--- Diff: core/sql/optimizer/OptLogRelExpr.cpp ---
@@ -2394,7 +2394,7 @@ Join::synthConstraints(NormWA * normWAPtr)
   (void) leftGA.hasCardConstraint(minLeft,maxLeft);
   (void) rightGA.hasCardConstraint(minRight,maxRight);
 
-  if (maxLeft == 1)
+  if (maxLeft == 1 || isIndexJoin_)
--- End diff --

What happens in the case of joining a non-unique index to a base table? 
Would we want to have probe caches then?


---


[GitHub] incubator-trafodion pull request #1255: [TRAFODION-2762] Allow UPDATE STATS ...

2017-10-03 Thread DaveBirdsall
GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/incubator-trafodion/pull/1255

[TRAFODION-2762] Allow UPDATE STATS to create sample tables regardless

This change causes UPDATE STATISTICS to always set CQD 
ALLOW_NULLABLE_UNIQUE_KEY_CONSTRAINT 'ON' and CQD CONTROL QUERY DEFAULT 
CAT_ERROR_ON_NOTNULL_STOREBY 'OFF'.

This allows UPDATE STATISTICS to create a sample table with a nullable 
primary key (the first CQD) or a nullable STORE BY (the second CQD). In this 
way, the user does not need to remember to set these CQDs when doing UPDATE 
STATISTICS on a table that was created with either of these experimental 
features.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/DaveBirdsall/incubator-trafodion 
nullableKeyUstatBug

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-trafodion/pull/1255.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1255


commit 2f49cda944da82ff8cc411b324d1000b8b9795d0
Author: Dave Birdsall 
Date:   2017-10-03T20:38:39Z

[TRAFODION-2762] Allow UPDATE STATS to create sample tables regardless of 
CQD




---


[GitHub] incubator-trafodion pull request #1254: [TRAFODION-2719] Check for truncatio...

2017-10-03 Thread zellerh
GitHub user zellerh opened a pull request:

https://github.com/apache/incubator-trafodion/pull/1254

[TRAFODION-2719] Check for truncation and four more fixes

This pull request combines five fixes, each in its own commit.

**[TRAFODION-2719] Check for truncation of character columns**

When inserting into a UTF-8 character column with a fixed number
of characters, do a check whether the source string contains no
more than the allowed number of characters (just checking the
byte length is not enough). This bug does not show when using
sqlci, since sqlci does its own check before calling the
executor. It only happens when using JDBC or trafci.

- core/sql/cli/CliExpExchange.cpp
- core/sql/exp/exp_expr.h
- core/sql/generator/GenExpGenerator.cpp
- core/sql/sqlcomp/DefaultConstants.h
- core/sql/sqlcomp/nadefaults.cpp


**[TRAFODION-2736] Missing predicates on salt columns**

In some cases, when we generate an index join (join of
an alternate index with the clustering index), we lost
the generated predicates on computed columns such as
salt and division. 

- core/sql/optimizer/RelScan.h
- partial: core/sql/optimizer/TransRule.cpp


**[TRAFODION-2751] Unnecessary PROBE_CACHE in an index join**

An index join produces unique matches from the index, so
a probe cache would be a waste of resources. Added a check
to suppress probe cache for index joins.

- core/sql/optimizer/OptLogRelExpr.cpp
- partial: core/sql/optimizer/TransRule.cpp
- core/sql/regress/core/EXPECTED005.SB

**[TRAFODION-2752] Fix error message when UDR method not found**

The error message displayed only the signature, but not the
method name.

- core/sql/regress/udr/EXPECTED100.SB
- core/sql/regress/udr/TEST100
- core/sql/sqlcomp/CmpSeabaseDDLroutine.cpp


**[TRAFODION-2761] Error when all TMUDF columns are eliminated**

Fix a couple of places where we would core dump when all
columns of the input table of a TMUDF were eliminated. Also
removed an unused NAColumnArray method that has a bug.

- core/sql/executor/ExUdr.cpp
- core/sql/generator/GenUdr.cpp
- core/sql/optimizer/NAColumn.cpp
- core/sql/optimizer/NAColumn.h
- core/sql/regress/udr/EXPECTED001
- core/sql/regress/udr/TEST001
- core/sql/regress/udr/TEST001_Sessionize.java


**Other fixes:**

Change the swjdbc script to pick up the Trafodion version from
the current environment, not when we run install_traf_components.

- core/sqf/sql/scripts/install_traf_components


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zellerh/incubator-trafodion bug/2719

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-trafodion/pull/1254.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1254


commit 279a9fd983bbd27eeacd5e084e1bf6b339cdd7ef
Author: Hans Zeller 
Date:   2017-10-03T17:00:50Z

[TRAFODION-2719] Check for truncation of character columns

When inserting into a UTF-8 character column with a fixed number
of characters, do a check whether the source string contains no
more than the allowed number of characters (just checking the
byte length is not enough). This bug does not show when using
sqlci, since sqlci does its own check before calling the
executor. It only happens when using JDBC or trafci.

commit 4327078e805617293c7a7ae028763b0949f6ec9f
Author: Hans Zeller 
Date:   2017-10-03T17:02:04Z

[TRAFODION-2736] Missing predicates on salt columns

In some cases, when we generate an index join (join of
an alternate index with the clustering index), we lost
the generated predicates on computed columns such as
salt and division.

commit 839f222356ed482215d876414761b4f1fcd54e59
Author: Hans Zeller 
Date:   2017-10-03T17:02:52Z

[TRAFODION-2751] Unnecessary PROBE_CACHE in an index join

An index join produces unique matches from the index, so
a probe cache would be a waste of resources. Added a check
to suppress probe cache for index joins.

commit 4b1c64c892e68cfbecd1f87bc7ddc5327c52723c
Author: Hans Zeller 
Date:   2017-10-03T17:03:40Z

[TRAFODION-2752] Fix error message when UDR method not found

The error message displayed only the signature, but not the
method name.

commit 4e7ad3aa967a60d4a212074e962f0f22a811eaf0
Author: Hans Zeller 
Date:   2017-10-03T17:04:12Z

[TRAFODION-2761] Error when all TMUDF columns are eliminated

Fix a couple of places where we would core dump when all
columns of the 

[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread DaveBirdsall
Github user DaveBirdsall commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1253#discussion_r142454172
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -292,8 +292,85 @@ static short genUpdExpr(
   return 0;
 }
 
-static short genUpdConstraintExpr(Generator *generator)
+// Used to generate update or insert constraint expressions for update 
operators
+static short genUpdConstraintExpr(Generator * generator,
+  ItemExpr * constrTree,
+  const ValueIdSet & constraintColumns,
+  ValueIdArray & targetRecExprArray,
+  ex_expr ** targetExpr /* out */)
 {
+  ExpGenerator * expGen = generator->getExpGenerator();
+
+  // The Attributes for the table columns refer to the old values of the 
column.
+  // The constraints must operate on the new values, though. So we must do 
a
+  // switcheroo on the Attributes for the update expression. The target 
value IDs
+  // come from targetRecExprArray.
+
+  ValueIdList savedSourceVIDlist;
+  NAList savedSourceAttrsList(generator->wHeap());
+
+  for (ValueId sourceValId = constraintColumns.init();
+   constraintColumns.next(sourceValId);
+   constraintColumns.advance(sourceValId))
+{
+  NAColumn * sourceCol = 
((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
+  ValueId targetValId;
+  NAColumn *targetCol = NULL;
--- End diff --

Just noticed something that could be improved. It would be better to set 
targetCol to NULL just inside the "for" loop, so that the "if" at line 329 
would not be taken. It's not an observable bug, though, because even if on the 
second time around, targetCol were pointing to an NAColumn from the first time 
around, the other comparisons at line 330 onward would come out false. 
Nevertheless, making the change would make it cleaner.


---


[GitHub] incubator-trafodion pull request #1253: [TRAFODION-1610][TRAFODION-2630] Gen...

2017-10-03 Thread DaveBirdsall
GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/incubator-trafodion/pull/1253

[TRAFODION-1610][TRAFODION-2630] Generate constr exprs on update operators

This set of changes plugs a hole in our implementation of 
update/upsert/merge operators. It completes work started by JIRA TRAFODION-1575.

This set of changes adds logic to the generator to generate update 
constraint check and insert constraint check expressions for update operators. 
(The update constraint check handles the run-time update path; the insert 
constraint check handles the run-time insert path for upsert and merge; it is 
absent for vanilla update and for merges that lack a "when not matched" clause.)

This set of changes also removes temporary code added long ago that 
transformed an UPDATE to an INSERT + a DELETE when constraint expressions were 
present. (The INSERT operator had a correct implementation of constraint 
checking.) This should make many non-key-column UPDATEs faster, and will also 
eliminate the possibility of HBase timestamp collisions because the INSERT 
happens within a millisecond of the DELETE.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/DaveBirdsall/incubator-trafodion Trafodion1610

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-trafodion/pull/1253.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1253


commit c22b6d10fdd3065a948163d64a20c3039128474c
Author: Dave Birdsall 
Date:   2017-10-03T16:09:29Z

[TRAFODION-1610][TRAFODION-2630] Generate constr exprs on update operators




---


[GitHub] incubator-trafodion pull request #1246: [TRAFODION-2645] First draft of a re...

2017-10-03 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-trafodion/pull/1246


---


[GitHub] incubator-trafodion pull request #1248: [trafodion-2728] - bug fix - SHOWSTA...

2017-10-03 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-trafodion/pull/1248


---