You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by selvaganesang <gi...@git.apache.org> on 2015/07/31 03:26:50 UTC
[GitHub] incubator-trafodion pull request: [TRAFODION-1423] Indexes on traf...
GitHub user selvaganesang opened a pull request:
https://github.com/apache/incubator-trafodion/pull/37
[TRAFODION-1423] Indexes on trafodion should be created in aligned ro…
…w format by default
A new CQD TRAF_INDEX_ALIGNED_ROW_FORMAT is set to ON by default.
The indexes are created in the aligned row format when this
CQD is set to ON and the table has no serialized column.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/selvaganesang/incubator-trafodion index_format
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/37.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 #37
----
commit 38327245fcf4d02191b3386f618d28d4397fbf59
Author: Selva Govindarajan <gs...@dev02.trafodion.org>
Date: 2015-07-31T01:14:42Z
[TRAFODION-1423] Indexes on trafodion should be created in aligned row format by default
A new CQD TRAF_INDEX_ALIGNED_ROW_FORMAT is set to ON by default.
The indexes are created in the aligned row format when this
CQD is set to ON and the table has no serialized column.
----
---
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-1423] Indexes on traf...
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/37#discussion_r36137159
--- Diff: core/sql/optimizer/NAFileSet.h ---
@@ -72,6 +72,13 @@ enum FileOrganizationEnum
KEY_SEQUENCED_FILE,
HASH_FILE
};
+
+enum RowFormatEnum {
+ SQLMX_UNKNOWN_FORMAT,
+ SQLMX_ROW_FORMAT,
+ SQLMX_ALIGNED_ROW_FORMAT,
+ SQLMX_HBASE_FORMAT
--- End diff --
SQLMX_ALIGNED_ROW_FORMAT is already used in NATable.h for a flag. Should we avoid this conflict (renaming either one). Also, I'm wondering whether this should go into ComSmallDefs, since it is fairly fundamental.
---
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-1423] Indexes on traf...
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/37#discussion_r36138238
--- Diff: core/sql/optimizer/NATable.h ---
@@ -576,7 +576,17 @@ class NATable : public NABasicObject
}
NABoolean isSQLMXAlignedTable() const
- { return (flags_ & SQLMX_ALIGNED_ROW_FORMAT) != 0; }
+ {
--- End diff --
Now that we have file formats per index, we should move this method from NATable to NAFileSet, for example as NAFileSet::isSQLMXAlignedFormat(). Instead of calling this method on an NATable *t, the caller should then call t->getClusteringIndex()->isSQLMXAlignedFormat(). I would also have some comments on the implementation below, but they are irrelevant if you replace this method.
---
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-1423] Indexes on traf...
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/37#discussion_r36138363
--- Diff: core/sql/optimizer/NATable.h ---
@@ -808,6 +824,9 @@ class NATable : public NABasicObject
NABoolean getRegionsNodeName(Int32 partns, ARRAY(const char *)& nodeNames) const;
private:
+ NABoolean getSQLMXAlignedTable() const
--- End diff --
Maybe remove this method and the flag and put it into NAFileSet?
---
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-1423] Indexes on traf...
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/37#discussion_r36086954
--- Diff: core/sql/generator/GenRelScan.cpp ---
@@ -2203,10 +2203,46 @@ short HbaseAccess::codeGen(Generator * generator)
ExpTupleDesc::SQLMX_KEY_FORMAT);
const ValueIdList &retColumnList = retColRefSet_;
+ // Always get the index name -- it will be the base tablename for
+ // primary access if it is trafodion table.
+ char * tablename = NULL;
+ char * snapshotName = NULL;
+ LatestSnpSupportEnum latestSnpSupport= latest_snp_supported;
+ if ((getTableDesc()->getNATable()->isHbaseRowTable()) ||
+ (getTableDesc()->getNATable()->isHbaseCellTable()))
+ {
+ tablename =
+ space->AllocateAndCopyToAlignedSpace(
+ GenGetQualifiedName(getTableName().getQualifiedNameObj().getObjectName()), 0);
+ latestSnpSupport = latest_snp_not_trafodion_table;
+ }
+ else
+ {
+ if (getIndexDesc() && getIndexDesc()->getNAFileSet())
+ {
+ tablename = space->AllocateAndCopyToAlignedSpace(GenGetQualifiedName(getIndexDesc()->getNAFileSet()->getFileSetName()), 0);
+ if (getIndexDesc()->isClusteringIndex())
+ {
+ //base table
+ snapshotName = (char*)getTableDesc()->getNATable()->getSnapshotName() ;
+ if (snapshotName == NULL)
+ latestSnpSupport = latest_snp_no_snapshot_available;
+ }
+ else
+ latestSnpSupport = latest_snp_index_table;
--- End diff --
Do we support the case where the base table has no snapshot defined on it but an index on the table does have a snapshot?
---
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-1423] Indexes on traf...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/37#discussion_r36128049
--- Diff: core/sql/generator/GenRelScan.cpp ---
@@ -2203,10 +2203,46 @@ short HbaseAccess::codeGen(Generator * generator)
ExpTupleDesc::SQLMX_KEY_FORMAT);
const ValueIdList &retColumnList = retColRefSet_;
+ // Always get the index name -- it will be the base tablename for
+ // primary access if it is trafodion table.
+ char * tablename = NULL;
+ char * snapshotName = NULL;
+ LatestSnpSupportEnum latestSnpSupport= latest_snp_supported;
+ if ((getTableDesc()->getNATable()->isHbaseRowTable()) ||
+ (getTableDesc()->getNATable()->isHbaseCellTable()))
+ {
+ tablename =
+ space->AllocateAndCopyToAlignedSpace(
+ GenGetQualifiedName(getTableName().getQualifiedNameObj().getObjectName()), 0);
+ latestSnpSupport = latest_snp_not_trafodion_table;
+ }
+ else
+ {
+ if (getIndexDesc() && getIndexDesc()->getNAFileSet())
+ {
+ tablename = space->AllocateAndCopyToAlignedSpace(GenGetQualifiedName(getIndexDesc()->getNAFileSet()->getFileSetName()), 0);
+ if (getIndexDesc()->isClusteringIndex())
+ {
+ //base table
+ snapshotName = (char*)getTableDesc()->getNATable()->getSnapshotName() ;
+ if (snapshotName == NULL)
+ latestSnpSupport = latest_snp_no_snapshot_available;
+ }
+ else
+ latestSnpSupport = latest_snp_index_table;
--- End diff --
It's outside of this change, but I'm curious: How in general do we guarantee consistency of a table snapshot with index snapshots?
---
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-1423] Indexes on traf...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/37#discussion_r36102842
--- Diff: core/sql/generator/GenRelScan.cpp ---
@@ -2203,10 +2203,46 @@ short HbaseAccess::codeGen(Generator * generator)
ExpTupleDesc::SQLMX_KEY_FORMAT);
const ValueIdList &retColumnList = retColRefSet_;
+ // Always get the index name -- it will be the base tablename for
+ // primary access if it is trafodion table.
+ char * tablename = NULL;
+ char * snapshotName = NULL;
+ LatestSnpSupportEnum latestSnpSupport= latest_snp_supported;
+ if ((getTableDesc()->getNATable()->isHbaseRowTable()) ||
+ (getTableDesc()->getNATable()->isHbaseCellTable()))
+ {
+ tablename =
+ space->AllocateAndCopyToAlignedSpace(
+ GenGetQualifiedName(getTableName().getQualifiedNameObj().getObjectName()), 0);
+ latestSnpSupport = latest_snp_not_trafodion_table;
+ }
+ else
+ {
+ if (getIndexDesc() && getIndexDesc()->getNAFileSet())
+ {
+ tablename = space->AllocateAndCopyToAlignedSpace(GenGetQualifiedName(getIndexDesc()->getNAFileSet()->getFileSetName()), 0);
+ if (getIndexDesc()->isClusteringIndex())
+ {
+ //base table
+ snapshotName = (char*)getTableDesc()->getNATable()->getSnapshotName() ;
+ if (snapshotName == NULL)
+ latestSnpSupport = latest_snp_no_snapshot_available;
+ }
+ else
+ latestSnpSupport = latest_snp_index_table;
--- End diff --
This part of the code is moved here from few lines below in the same function just to get the tablename. The snapshot functionality is not affected by this change. Yes. We need to figure out the consequences if the snapshot for the index is not created, when table has snapshot and vice versa. But, it is outside the scope of this change
---
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-1423] Indexes on traf...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/37#discussion_r36128340
--- Diff: core/sql/optimizer/NATable.cpp ---
@@ -7596,6 +7617,19 @@ NABoolean NATable::getHbaseTableInfo(Int32& hbtIndexLevels, Int32& hbtBlockSize
}
+NABoolean NATable::isSQLMXAlignedTable(char *objectName) const
+{
+ if (qualifiedName_.getText().compareTo(objectName, NAString::exact) == 0)
+ return getSQLMXAlignedTable();
+ else {
+ for (CollIndex i = 0; i < indexes_.entries(); i++) {
+ if (indexes_[i]->getFileSetName().getQualifiedNameAsAnsiString().compareTo(objectName, NAString::exact) == 0)
+ return indexes_[i]->isSqlmxAlignedRowFormat();
+ }
+ }
+ return getSQLMXAlignedTable();
--- End diff --
Should we ever get here? (If we do, is this an internal error?)
---
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-1423] Indexes on traf...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/37
---
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-1423] Indexes on traf...
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/37#discussion_r36138975
--- Diff: core/sql/sqlcomp/CmpSeabaseDDLtable.cpp ---
@@ -8358,6 +8358,19 @@ desc_struct * CmpSeabaseDDL::getSeabaseUserTableDesc(const NAString &catName,
Lng32 keyColCount = *(Lng32*)vi->get(6);
Lng32 nonKeyColCount = *(Lng32*)vi->get(7);
Lng32 idxNumSaltPartns = *(Lng32*)vi->get(8);
+ char * format = vi->get(9);
+ Lng32 idxRowFormat;
+
+ if (memcmp(format, COM_ALIGNED_FORMAT_LIT, 2) == 0)
--- End diff --
Not sure I understand this: This gets the row format from the TABLES table, so the index row format is the same for all the indexes, is that correct? Wouldn't we need to do that per index? For example, older tables could have an aligned base table and unaligned indexes, while newer tables and indexes could both be aligned?
---
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.
---