You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by sureshsubbiah <gi...@git.apache.org> on 2015/10/06 23:09:39 UTC
[GitHub] incubator-trafodion pull request: [TRAFODION-1482] [TRAFODION-1474...
GitHub user sureshsubbiah opened a pull request:
https://github.com/apache/incubator-trafodion/pull/107
[TRAFODION-1482] [TRAFODION-1474] [TRAFODION-1279]
Please see JIRA issues for information on each fix.
The files are
1482: HTableClient.java
1474: GenUdr.cpp
1279:NaColumn.h, NAColumn.cpp, CmpSeabaseDDLtable.cpp
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sureshsubbiah/incubator-trafodion jira_bugs3
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/107.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 #107
----
commit a1f99cb76183afb27a47a7517414d0f7ca7a2b3d
Author: Suresh Subbiah <su...@apache.org>
Date: 2015-10-06T21:06:34Z
[TRAFODION-1482] [TRAFODION-1474] [TRAFODION-1279]
Please see JIRA issues for information on each fix.
The files are
1482: HTableClient.java
1474: GenUdr.cpp
1279: NAColumn.h, NAColumn.cpp, CmpSeabaseDDLtable.cpp
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1482] [TRAFODION-1474...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/107#discussion_r41326041
--- Diff: core/sql/sqlcomp/CmpSeabaseDDLtable.cpp ---
@@ -4609,7 +4611,7 @@ void CmpSeabaseDDL::alterSeabaseTableAddColumn(
}
Int64 objUID = naTable->objectUid().castToInt64();
- str_sprintf(query, "insert into %s.\"%s\".%s values (%Ld, '%s', %d, '%s', %d, '%s', %d, %d, %d, %d, %d, '%s', %d, %d, '%s', %d, '%s', '%s', '%s', '%d', '%s', '%s', %Ld )",
+ str_sprintf(query, "insert into %s.\"%s\".%s values (%Ld, '%s', %d, '%s', %d, '%s', %d, %d, %d, %d, %d, '%s', %d, %d, '%s', %d, '%s', '%s', '%s', '%u', '%s', '%s', %Ld )",
--- End diff --
Shouldn't this be %d?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1482] [TRAFODION-1474...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/107#discussion_r41327949
--- Diff: core/sql/optimizer/NAColumn.cpp ---
@@ -749,6 +749,27 @@ Lng32 NAColumnArray::getOffset(Lng32 position) const
return result;
}
+
+Lng32 NAColumnArray::getMaxTrafHbaseColQualifier() const
+{
+ NAColumn *column;
+ char * colQualPtr;
+ Lng32 colQualLen;
+ Int64 colQVal;
+ Lng32 maxVal = 0;
+
+ for (CollIndex i = 0; i < entries(); i++)
+ {
+ column = (*this)[i];
+ colQualPtr = (char*)column->getHbaseColQual().data();
+ colQualLen = column->getHbaseColQual().length();
+ colQVal = str_atoi(colQualPtr, colQualLen);
+ if (colQVal > maxVal)
+ maxVal = colQVal ;
+ }
+ return colQVal;
--- End diff --
Thanks, sounds great.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1482] [TRAFODION-1474...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/107#discussion_r41326027
--- Diff: core/sql/optimizer/NAColumn.cpp ---
@@ -749,6 +749,27 @@ Lng32 NAColumnArray::getOffset(Lng32 position) const
return result;
}
+
+Lng32 NAColumnArray::getMaxTrafHbaseColQualifier() const
+{
+ NAColumn *column;
+ char * colQualPtr;
+ Lng32 colQualLen;
+ Int64 colQVal;
+ Lng32 maxVal = 0;
+
+ for (CollIndex i = 0; i < entries(); i++)
+ {
+ column = (*this)[i];
+ colQualPtr = (char*)column->getHbaseColQual().data();
+ colQualLen = column->getHbaseColQual().length();
+ colQVal = str_atoi(colQualPtr, colQualLen);
+ if (colQVal > maxVal)
+ maxVal = colQVal ;
+ }
+ return colQVal;
--- End diff --
Shouldn't this return maxVal? It probably works with colQVal, since the column qualifiers are in ascending order. Also, what about initializing maxVal with -1? We don't support tables with no columns, so this hair-splitting and a theoretical argument.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1482] [TRAFODION-1474...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/107
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1482] [TRAFODION-1474...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/107#discussion_r41327749
--- Diff: core/sql/sqlcomp/CmpSeabaseDDLtable.cpp ---
@@ -4609,7 +4611,7 @@ void CmpSeabaseDDL::alterSeabaseTableAddColumn(
}
Int64 objUID = naTable->objectUid().castToInt64();
- str_sprintf(query, "insert into %s.\"%s\".%s values (%Ld, '%s', %d, '%s', %d, '%s', %d, %d, %d, %d, %d, '%s', %d, %d, '%s', %d, '%s', '%s', '%s', '%d', '%s', '%s', %Ld )",
+ str_sprintf(query, "insert into %s.\"%s\".%s values (%Ld, '%s', %d, '%s', %d, '%s', %d, %d, %d, %d, %d, '%s', %d, %d, '%s', %d, '%s', '%s', '%s', '%u', '%s', '%s', %Ld )",
--- End diff --
It's just that maxColQual is defined as a Lng32 on line 4599 and that is an int, not an unsigned int.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1482] [TRAFODION-1474...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/107#discussion_r41327845
--- Diff: core/sql/optimizer/NAColumn.cpp ---
@@ -749,6 +749,27 @@ Lng32 NAColumnArray::getOffset(Lng32 position) const
return result;
}
+
+Lng32 NAColumnArray::getMaxTrafHbaseColQualifier() const
+{
+ NAColumn *column;
+ char * colQualPtr;
+ Lng32 colQualLen;
+ Int64 colQVal;
+ Lng32 maxVal = 0;
+
+ for (CollIndex i = 0; i < entries(); i++)
+ {
+ column = (*this)[i];
+ colQualPtr = (char*)column->getHbaseColQual().data();
+ colQualLen = column->getHbaseColQual().length();
+ colQVal = str_atoi(colQualPtr, colQualLen);
+ if (colQVal > maxVal)
+ maxVal = colQVal ;
+ }
+ return colQVal;
--- End diff --
Thanks for catching this. I will fix the return value in a minute. The smallest colQualifier we add is 1 for a Traf table. We don't seem to add colQualifier with value 0. Returning a negative value will mean that it will have to be checked by the caller.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1482] [TRAFODION-1474...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/107#discussion_r41327452
--- Diff: core/sql/sqlcomp/CmpSeabaseDDLtable.cpp ---
@@ -4609,7 +4611,7 @@ void CmpSeabaseDDL::alterSeabaseTableAddColumn(
}
Int64 objUID = naTable->objectUid().castToInt64();
- str_sprintf(query, "insert into %s.\"%s\".%s values (%Ld, '%s', %d, '%s', %d, '%s', %d, %d, %d, %d, %d, '%s', %d, %d, '%s', %d, '%s', '%s', '%s', '%d', '%s', '%s', %Ld )",
+ str_sprintf(query, "insert into %s.\"%s\".%s values (%Ld, '%s', %d, '%s', %d, '%s', %d, %d, %d, %d, %d, '%s', %d, %d, '%s', %d, '%s', '%s', '%s', '%u', '%s', '%s', %Ld )",
--- End diff --
This was intentional. We are using this value as an unsigned int in the generator. I athought we might get into some kind of overflow situation if the number of columns is in between signed_int_max and unsigned_int_max.
Code snippet from HbaseAccess:createHbaseColId()
Int64 colQval = str_atoi(colQualPtr, colQualLen);
if (colQval <= UCHAR_MAX)
{
unsigned char c = (unsigned char)colQval;
cid.append((char*)&c, 1);
}
else if (colQval <= USHRT_MAX)
{
unsigned short s = (unsigned short)colQval;
cid.append((char*)&s, 2);
}
else if (colQval <= ULONG_MAX)
{
Lng32 l = (Lng32)colQval;
cid.append((char*)&l, 4);
}
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1482] [TRAFODION-1474...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/107#discussion_r41433413
--- Diff: core/sql/sqlcomp/CmpSeabaseDDLtable.cpp ---
@@ -4609,7 +4611,7 @@ void CmpSeabaseDDL::alterSeabaseTableAddColumn(
}
Int64 objUID = naTable->objectUid().castToInt64();
- str_sprintf(query, "insert into %s.\"%s\".%s values (%Ld, '%s', %d, '%s', %d, '%s', %d, %d, %d, %d, %d, '%s', %d, %d, '%s', %d, '%s', '%s', '%s', '%d', '%s', '%s', %Ld )",
+ str_sprintf(query, "insert into %s.\"%s\".%s values (%Ld, '%s', %d, '%s', %d, '%s', %d, %d, %d, %d, %d, '%s', %d, %d, '%s', %d, '%s', '%s', '%s', '%u', '%s', '%s', %Ld )",
--- End diff --
You are right Hans. I misread the line in Platform.h. Let me fix this in my next checkin.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---