[GitHub] incubator-trafodion pull request #950: [TRAFODION-2471] pyinstaller: error c...

[mkby]

GitHub user mkby opened a pull request:

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

[TRAFODION-2471] pyinstaller: error command when grant privilege in HBase.

The old command doesn't work.
also change the owner of HDFS directory /user/trafodion

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

$ git pull https://github.com/mkby/incubator-trafodion 2471

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

https://github.com/apache/incubator-trafodion/pull/950.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 #950


commit 0c3cdbdf0f487c79728fdf1e17a63b9e3c07b571
Author: Eason 
Date:   2017-02-09T06:48:10Z

[TRAFODION-2471] pyinstaller: error command when grant privilege in HBase.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[sandhyasun]

Github user sandhyasun commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100206113
  
--- Diff: core/sql/optimizer/BindRelExpr.cpp ---
@@ -10600,34 +10600,45 @@ Upsert is also converted into merge when 
TRAF_UPSERT_MODE is set to MERGE and
 there are omitted cols with default values in case of aligned format table 
or 
 omitted current timestamp cols in case of non-aligned row format
 */
-NABoolean Insert::isUpsertThatNeedsMerge(NABoolean isAlignedRowFormat, 
NABoolean omittedDefaultCols,
-   NABoolean 
omittedCurrentDefaultClassCols) const
+NABoolean Insert::isUpsertThatNeedsTransformation(NABoolean 
isAlignedRowFormat, 
+  NABoolean 
omittedDefaultCols,
+  NABoolean 
omittedCurrentDefaultClassCols,
+  NABoolean ) const
 {
-  // The necessary conditions to convert upsert to merge and
+  // The necessary conditions to transform upsert 
   if (isUpsert() && 
   (NOT getIsTrafLoadPrep()) && 
-  (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
getTableDesc()->hasIdentityColumnInClusteringKey())) && 
+  (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
+getTableDesc()->hasIdentityColumnInClusteringKey())) && 
   (NOT 
(getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
-// table has secondary indexes or
-(getTableDesc()->hasSecondaryIndexes() ||
-  // CQD is set to MERGE  
-  ((CmpCommon::getDefault(TRAF_UPSERT_MODE) == DF_MERGE) &&
+  // table has secondary indexes or
+  (getTableDesc()->hasSecondaryIndexes() ))
+{
+  toMerge = FALSE;
+  return TRUE;
+}
+  else if (isUpsert() &&   (NOT getIsTrafLoadPrep()) && 
+   (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
getTableDesc()->hasIdentityColumnInClusteringKey())) && 
+   (NOT 
(getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
+   // CQD is set to MERGE  
+   ((CmpCommon::getDefault(TRAF_UPSERT_MODE) == DF_MERGE) &&
 // omitted current default columns with non-aligned row format 
tables
 // or omitted default columns with aligned row format tables 
 (((NOT isAlignedRowFormat) && omittedCurrentDefaultClassCols) 
||
--- End diff --

Yes I am reworking this and making the code clearer with new comments.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[zellerh]

Github user zellerh commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100204362
  
--- Diff: core/sql/optimizer/BindRelExpr.cpp ---
@@ -10600,34 +10600,45 @@ Upsert is also converted into merge when 
TRAF_UPSERT_MODE is set to MERGE and
 there are omitted cols with default values in case of aligned format table 
or 
 omitted current timestamp cols in case of non-aligned row format
 */
-NABoolean Insert::isUpsertThatNeedsMerge(NABoolean isAlignedRowFormat, 
NABoolean omittedDefaultCols,
-   NABoolean 
omittedCurrentDefaultClassCols) const
+NABoolean Insert::isUpsertThatNeedsTransformation(NABoolean 
isAlignedRowFormat, 
+  NABoolean 
omittedDefaultCols,
+  NABoolean 
omittedCurrentDefaultClassCols,
+  NABoolean ) const
 {
-  // The necessary conditions to convert upsert to merge and
+  // The necessary conditions to transform upsert 
   if (isUpsert() && 
   (NOT getIsTrafLoadPrep()) && 
-  (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
getTableDesc()->hasIdentityColumnInClusteringKey())) && 
+  (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
--- End diff --

Maybe add a comment here that tables with an identity or syskey column 
never encounter any duplicate rows on insert/upsert.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[zellerh]

Github user zellerh commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100203386
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -2855,8 +2859,8 @@ short HbaseInsert::codeGen(Generator *generator)
setVsbbInsert(TRUE);
   }
 
-  if ((isUpsert()) &&
- (getInsertType() == Insert::UPSERT_LOAD))
+  
+if (getInsertType() == Insert::UPSERT_LOAD)
--- End diff --

Just to make sure I understand: You removed the isUpsert() here because you 
know that for upserts that are VSBB inserts, the insert type is UPSERT_LOAD?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[zellerh]

Github user zellerh commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100202964
  
--- Diff: core/sql/generator/GenRelUpdate.cpp ---
@@ -2844,8 +2848,8 @@ short HbaseInsert::codeGen(Generator *generator)
  (noCheck()))
hbasescan_tdb->setHbaseSqlIUD(FALSE);
 
-  if (((getInsertType() == Insert::VSBB_INSERT_USER) && 
-   generator->oltOptInfo()->multipleRowsReturned()) ||
+  if (getInsertType() == Insert::VSBB_INSERT_USER) || isUpsert() 
)&& 
+   generator->oltOptInfo()->multipleRowsReturned())) ||
--- End diff --

A nit, there is an unnecessary pair of parentheses at the beginning of line 
2851 and the end of line 2852.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[zellerh]

Github user zellerh commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100204780
  
--- Diff: core/sql/optimizer/BindRelExpr.cpp ---
@@ -10600,34 +10600,45 @@ Upsert is also converted into merge when 
TRAF_UPSERT_MODE is set to MERGE and
 there are omitted cols with default values in case of aligned format table 
or 
 omitted current timestamp cols in case of non-aligned row format
 */
-NABoolean Insert::isUpsertThatNeedsMerge(NABoolean isAlignedRowFormat, 
NABoolean omittedDefaultCols,
-   NABoolean 
omittedCurrentDefaultClassCols) const
+NABoolean Insert::isUpsertThatNeedsTransformation(NABoolean 
isAlignedRowFormat, 
+  NABoolean 
omittedDefaultCols,
+  NABoolean 
omittedCurrentDefaultClassCols,
+  NABoolean ) const
 {
-  // The necessary conditions to convert upsert to merge and
+  // The necessary conditions to transform upsert 
   if (isUpsert() && 
   (NOT getIsTrafLoadPrep()) && 
-  (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
getTableDesc()->hasIdentityColumnInClusteringKey())) && 
+  (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
+getTableDesc()->hasIdentityColumnInClusteringKey())) && 
   (NOT 
(getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
-// table has secondary indexes or
-(getTableDesc()->hasSecondaryIndexes() ||
-  // CQD is set to MERGE  
-  ((CmpCommon::getDefault(TRAF_UPSERT_MODE) == DF_MERGE) &&
+  // table has secondary indexes or
+  (getTableDesc()->hasSecondaryIndexes() ))
+{
+  toMerge = FALSE;
+  return TRUE;
+}
+  else if (isUpsert() &&   (NOT getIsTrafLoadPrep()) && 
+   (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
getTableDesc()->hasIdentityColumnInClusteringKey())) && 
+   (NOT 
(getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
+   // CQD is set to MERGE  
+   ((CmpCommon::getDefault(TRAF_UPSERT_MODE) == DF_MERGE) &&
 // omitted current default columns with non-aligned row format 
tables
 // or omitted default columns with aligned row format tables 
 (((NOT isAlignedRowFormat) && omittedCurrentDefaultClassCols) 
||
--- End diff --

Maybe another short comment here that TRAF_UPSERT_MERGE controls the 
behavior of upserting an existing row with a subset of columns. ON means keep 
the column values of the existing row for the omitted columns (slower) and OFF 
means setting the omitted columns to default values (faster). Since there is no 
standard for UPSERT, either behavior could be considered "correct".


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #949: [TRAFODION-2475] Create table/index o...

[DaveBirdsall]

Github user DaveBirdsall commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/949#discussion_r100171432
  
--- Diff: core/sql/src/main/java/org/trafodion/sql/HBaseClient.java ---
@@ -294,15 +296,27 @@ private ChangeFlags setDescriptors(Object[] 
tableOptions,
break ;
case HBASE_COMPRESSION:
if (tableOption.equalsIgnoreCase("GZ"))
+  {// throws IOException
+  CompressionTest.testCompression(Algorithm.GZ);
colDesc.setCompressionType(Algorithm.GZ);
+  }
else if (tableOption.equalsIgnoreCase("LZ4"))
+  {   // throws IOException
+  CompressionTest.testCompression(Algorithm.LZ4);
colDesc.setCompressionType(Algorithm.LZ4);
+  }
else if (tableOption.equalsIgnoreCase("LZO"))
+  {   // throws IOException
+  CompressionTest.testCompression(Algorithm.LZO);
colDesc.setCompressionType(Algorithm.LZO);
+  }
else if (tableOption.equalsIgnoreCase("NONE"))
colDesc.setCompressionType(Algorithm.NONE);
else if (tableOption.equalsIgnoreCase("SNAPPY"))
+  {   // throws IOException
+  CompressionTest.testCompression(Algorithm.SNAPPY);
colDesc.setCompressionType(Algorithm.SNAPPY); 
+  }
--- End diff --

Should there be an exception if the tableOption isn't recognized? (Right 
now the code just ignores it.)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #949: [TRAFODION-2475] Create table/index o...

[DaveBirdsall]

Github user DaveBirdsall commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/949#discussion_r100171468
  
--- Diff: core/sql/src/main/java/org/trafodion/sql/HBaseClient.java ---
@@ -354,16 +368,28 @@ else if (tableOption.equalsIgnoreCase("PREFIX_TREE"))
returnStatus.setColumnDescriptorChanged();
break ;
case HBASE_COMPACT_COMPRESSION:
-   if (tableOption.equalsIgnoreCase("GZ"))
-   colDesc.setCompactionCompressionType(Algorithm.GZ);
-   else if (tableOption.equalsIgnoreCase("LZ4"))
+   if (tableOption.equalsIgnoreCase("GZ")) {
+  // throws IOException
+  CompressionTest.testCompression(Algorithm.GZ);
+   colDesc.setCompactionCompressionType(Algorithm.GZ); 
+  }
+   else if (tableOption.equalsIgnoreCase("LZ4")) {
+  // throws IOException
+  CompressionTest.testCompression(Algorithm.LZ4);
colDesc.setCompactionCompressionType(Algorithm.LZ4);
-   else if (tableOption.equalsIgnoreCase("LZO"))
+  }
+   else if (tableOption.equalsIgnoreCase("LZO")) {
+  // throws IOException
+  CompressionTest.testCompression(Algorithm.LZO);
colDesc.setCompactionCompressionType(Algorithm.LZO);
+  }
else if (tableOption.equalsIgnoreCase("NONE"))
colDesc.setCompactionCompressionType(Algorithm.NONE);
-   else if (tableOption.equalsIgnoreCase("SNAPPY"))
+   else if (tableOption.equalsIgnoreCase("SNAPPY")) {
+  // throws IOException
+  CompressionTest.testCompression(Algorithm.SNAPPY);
colDesc.setCompactionCompressionType(Algorithm.SNAPPY); 
+  }
--- End diff --

Same remark as above.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #949: [TRAFODION-2475] Create table/index o...

[sureshsubbiah]

GitHub user sureshsubbiah opened a pull request:

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

[TRAFODION-2475] Create table/index or alter does not work with unava…

…ilable compression options

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

$ git pull https://github.com/sureshsubbiah/incubator-trafodion compress1

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

https://github.com/apache/incubator-trafodion/pull/949.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 #949


commit a2b75756cabb6b0d8483ab4f55933e29055ba3d8
Author: Suresh Subbiah 
Date:   2017-02-08T20:45:45Z

[TRAFODION-2475] Create table/index or alter does not work with unavailable 
compression options




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[sandhyasun]

Github user sandhyasun commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100155118
  
--- Diff: core/sql/optimizer/BindRelExpr.cpp ---
@@ -10600,34 +10600,45 @@ Upsert is also converted into merge when 
TRAF_UPSERT_MODE is set to MERGE and
 there are omitted cols with default values in case of aligned format table 
or 
 omitted current timestamp cols in case of non-aligned row format
 */
-NABoolean Insert::isUpsertThatNeedsMerge(NABoolean isAlignedRowFormat, 
NABoolean omittedDefaultCols,
-   NABoolean 
omittedCurrentDefaultClassCols) const
+NABoolean Insert::isUpsertThatNeedsTransformation(NABoolean 
isAlignedRowFormat, 
+  NABoolean 
omittedDefaultCols,
+  NABoolean 
omittedCurrentDefaultClassCols,
+  NABoolean ) const
 {
-  // The necessary conditions to convert upsert to merge and
+  // The necessary conditions to transform upsert 
   if (isUpsert() && 
   (NOT getIsTrafLoadPrep()) && 
-  (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
getTableDesc()->hasIdentityColumnInClusteringKey())) && 
+  (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
+getTableDesc()->hasIdentityColumnInClusteringKey())) && 
   (NOT 
(getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
-// table has secondary indexes or
-(getTableDesc()->hasSecondaryIndexes() ||
-  // CQD is set to MERGE  
-  ((CmpCommon::getDefault(TRAF_UPSERT_MODE) == DF_MERGE) &&
+  // table has secondary indexes or
+  (getTableDesc()->hasSecondaryIndexes() ))
+{
+  toMerge = FALSE;
+  return TRUE;
+}
+  else if (isUpsert() &&   (NOT getIsTrafLoadPrep()) && 
+   (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
getTableDesc()->hasIdentityColumnInClusteringKey())) && 
+   (NOT 
(getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
+   // CQD is set to MERGE  
--- End diff --

Good point...Let me rework this to be more readable and may have to add a 
3rd check. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[sandhyasun]

Github user sandhyasun commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100151468
  
--- Diff: core/sql/executor/ExHbaseIUD.cpp ---
@@ -1042,7 +1104,10 @@ ExWorkProcRetcode 
ExHbaseAccessUpsertVsbbSQTcb::work()
 
numRetries_++;
return WORK_CALL_AGAIN;
- }
+
+step_ = PROCESS_INSERT_AND_CLOSE;
--- End diff --

Yes . Not needed. We transition to PROCESS_INSERT_AND_CLOSE only after 3 
tries. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[sandhyasun]

Github user sandhyasun commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100151343
  
--- Diff: core/sql/executor/ExHbaseIUD.cpp ---
@@ -1620,21 +1685,20 @@ ExWorkProcRetcode 
ExHbaseAccessBulkLoadPrepSQTcb::work()
   {
if (qparent_.up->isFull())
  return WORK_OK;
-   
+
if (returnUpdateExpr())
{
- ex_queue_entry * up_entry = qparent_.up->getTailEntry();
- 
- // allocate tupps where returned rows will be created
- if (allocateUpEntryTupps(
-  -1,
-  0,
-  hbaseAccessTdb().returnedTuppIndex_,
-  hbaseAccessTdb().returnUpdatedRowLen_,
-  FALSE,
-  ))
+ ex_queue_entry * up_entry = qparent_.up->getTailEntry();   
return rc;
- 
+// allocate tupps where returned rows will be created
+if (allocateUpEntryTupps(
--- End diff --

Removed this inadvertant bug. Thanks !



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[DaveBirdsall]

Github user DaveBirdsall commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100134742
  
--- Diff: core/sql/executor/ExHbaseIUD.cpp ---
@@ -1620,21 +1685,20 @@ ExWorkProcRetcode 
ExHbaseAccessBulkLoadPrepSQTcb::work()
   {
if (qparent_.up->isFull())
  return WORK_OK;
-   
+
if (returnUpdateExpr())
{
- ex_queue_entry * up_entry = qparent_.up->getTailEntry();
- 
- // allocate tupps where returned rows will be created
- if (allocateUpEntryTupps(
-  -1,
-  0,
-  hbaseAccessTdb().returnedTuppIndex_,
-  hbaseAccessTdb().returnUpdatedRowLen_,
-  FALSE,
-  ))
+ ex_queue_entry * up_entry = qparent_.up->getTailEntry();   
return rc;
- 
+// allocate tupps where returned rows will be created
+if (allocateUpEntryTupps(
--- End diff --

This statement can't be reached because of the return at 1692.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[DaveBirdsall]

Github user DaveBirdsall commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100134813
  
--- Diff: core/sql/executor/ExHbaseIUD.cpp ---
@@ -1620,21 +1685,20 @@ ExWorkProcRetcode 
ExHbaseAccessBulkLoadPrepSQTcb::work()
   {
if (qparent_.up->isFull())
  return WORK_OK;
-   
+
if (returnUpdateExpr())
{
- ex_queue_entry * up_entry = qparent_.up->getTailEntry();
- 
- // allocate tupps where returned rows will be created
- if (allocateUpEntryTupps(
-  -1,
-  0,
-  hbaseAccessTdb().returnedTuppIndex_,
-  hbaseAccessTdb().returnUpdatedRowLen_,
-  FALSE,
-  ))
+ ex_queue_entry * up_entry = qparent_.up->getTailEntry();   
return rc;
- 
+// allocate tupps where returned rows will be created
+if (allocateUpEntryTupps(
+ -1,
+ 0,
+ hbaseAccessTdb().returnedTuppIndex_,
+ hbaseAccessTdb().returnUpdatedRowLen_,
+ FALSE,
+ ))  
+  return 1;
--- End diff --

Should this be return rc?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[DaveBirdsall]

Github user DaveBirdsall commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100134622
  
--- Diff: core/sql/executor/ExHbaseIUD.cpp ---
@@ -1042,7 +1104,10 @@ ExWorkProcRetcode 
ExHbaseAccessUpsertVsbbSQTcb::work()
 
numRetries_++;
return WORK_CALL_AGAIN;
- }
+
+step_ = PROCESS_INSERT_AND_CLOSE;
--- End diff --

I think a bug might have crept in here. This statement can't be reached 
because of the return at line 1106.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #947: [TRAFODION-2472] disable transactiona...

[asfgit]

Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #948: [TRAFODION-2442] Small correction to ...

[asfgit]

Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[sureshsubbiah]

Github user sureshsubbiah commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100099385
  
--- Diff: core/sql/regress/executor/EXPECTED015.SB ---
@@ -1961,51 +1970,51 @@ TRAFODION_MERGE
 +>(select d.id from DEC1 d where d.codeValue = ?),  ? ) ;
 
 --- SQL command prepared.
->>execute explainIt;
+>>execute explainItEff;
 
 OPERATOR
 
 
-TRAFODION_MERGE 
+SEQUENCE
 
 --- 1 row(s) selected.
 >>
 >>prepare XX from UPSERT INTO DE (ID, dataElementConceptID, valueDomainID) 
 
 +>VALUES  (1,  (select d.id from DEC1 d where d.codeValue = ?),  3 ) ;
 
 --- SQL command prepared.
->>execute explainIt;
+>>execute explainItEff;
 
 OPERATOR
 
 
-TRAFODION_MERGE 
+SEQUENCE
 
 --- 1 row(s) selected.
 >>
 >>prepare XX from UPSERT INTO DE (ID, dataElementConceptID, valueDomainID) 
 
 +>VALUES  (?[10], ?[10], ?[10] ) ;
 
 --- SQL command prepared.
->>execute explainIt;
+>>execute explainItEff;
 
 OPERATOR
 
 
-TRAFODION_MERGE 
+SEQUENCE
--- End diff --

Test cases are nice and robust. I like them.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #946: TRAFODION-1562 Enable Vsbb upsert for...

[sureshsubbiah]

Github user sureshsubbiah commented on a diff in the pull request:

https://github.com/apache/incubator-trafodion/pull/946#discussion_r100098694
  
--- Diff: core/sql/optimizer/BindRelExpr.cpp ---
@@ -10600,34 +10600,45 @@ Upsert is also converted into merge when 
TRAF_UPSERT_MODE is set to MERGE and
 there are omitted cols with default values in case of aligned format table 
or 
 omitted current timestamp cols in case of non-aligned row format
 */
-NABoolean Insert::isUpsertThatNeedsMerge(NABoolean isAlignedRowFormat, 
NABoolean omittedDefaultCols,
-   NABoolean 
omittedCurrentDefaultClassCols) const
+NABoolean Insert::isUpsertThatNeedsTransformation(NABoolean 
isAlignedRowFormat, 
+  NABoolean 
omittedDefaultCols,
+  NABoolean 
omittedCurrentDefaultClassCols,
+  NABoolean ) const
 {
-  // The necessary conditions to convert upsert to merge and
+  // The necessary conditions to transform upsert 
   if (isUpsert() && 
   (NOT getIsTrafLoadPrep()) && 
-  (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
getTableDesc()->hasIdentityColumnInClusteringKey())) && 
+  (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
+getTableDesc()->hasIdentityColumnInClusteringKey())) && 
   (NOT 
(getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
-// table has secondary indexes or
-(getTableDesc()->hasSecondaryIndexes() ||
-  // CQD is set to MERGE  
-  ((CmpCommon::getDefault(TRAF_UPSERT_MODE) == DF_MERGE) &&
+  // table has secondary indexes or
+  (getTableDesc()->hasSecondaryIndexes() ))
+{
+  toMerge = FALSE;
+  return TRUE;
+}
+  else if (isUpsert() &&   (NOT getIsTrafLoadPrep()) && 
+   (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && 
getTableDesc()->hasIdentityColumnInClusteringKey())) && 
+   (NOT 
(getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
+   // CQD is set to MERGE  
--- End diff --

The conditions in IF and ELSE IF are duplicated, making it harder to read. 
Please consider using a local variable to store the result of common 
conditions. Also, it is not clear what should happen if we have both IM (no 
syskey etc.) and default values in the upsert. With current code we will do the 
new efficient transformation. I suppose that means the new efficient 
transformation can handle the missing default values case too. Is the else if 
case used as fallback ? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---