[GitHub] trafodion pull request #1759: [TRAFODION-3237] Fix incorrect PCode optimizat...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1759

[TRAFODION-3237] Fix incorrect PCode optimization

When an interval literal was referenced twice or more in an INSERT/SELECT 
in expressions with different INTERVAL types, the PCode optimizer was 
incorrectly treating the code to produce the interval literal value as a common 
subexpression, so the INSERT/SELECT would insert incorrect values. 

This has been fixed. The PCode optimizer common subexpression elimination 
logic now checks for commonality in datatype, scale and precision for INTERVAL 
conversions.

A test case that demonstrates the problem (with correct behavior with the 
fix) has been added to core/TEST038.



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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3237

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

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


commit 8b4e533d3082379dc090e7efeefab80741334d00
Author: Dave Birdsall 
Date:   2018-12-10T23:32:00Z

[TRAFODION-3237] Fix incorrect PCode optimization




---

[GitHub] trafodion pull request #1756: [TRAFODION-3243] Avoid dereference of deleted ...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1756

[TRAFODION-3243] Avoid dereference of deleted NAString in UPDATE STATISTICS

In the HSColGroupStruct destructor, move the call to 
HSColGroupStruct::freeISMemory up to the front, before the "delete colNames" 
statement. The freeISMemory method dereferences colNames. Before this fix, this 
could cause cores.

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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3243

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

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


commit 95380a07e2e4f2aec45d9f0b5a92378cc13da33f
Author: Dave Birdsall 
Date:   2018-12-05T21:15:08Z

[TRAFODION-3243] Avoid dereference of deleted NAString in UPDATE STATISTICS




---

[GitHub] trafodion pull request #1750: [TRAFODION-3238] ODBC can not connect to serve...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1750#discussion_r237702542
  
--- Diff: win-odbc64/security_dll/native/source/secpwd.cpp ---
@@ -217,8 +221,25 @@ SecPwd::SecPwd(const char *dir, const char* fileName,
 if(stat(certDir,&st) != 0)
 throw SecurityException(DIR_NOTFOUND, (char *)certDir);
 
-certFile = buildName(certDir, fileName, serverName, CER);
-activeCertFile = buildName(certDir, activeFileName, serverName, 
ACTIVE_CER);
+if (lcid == 0x804) // if local charset is not utf8
--- End diff --

No... I suppose we'll let the marketplace tell us.


---

[GitHub] trafodion pull request #1750: [TRAFODION-3238] ODBC can not connect to serve...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1750#discussion_r237304832
  
--- Diff: win-odbc64/security_dll/native/source/secpwd.cpp ---
@@ -217,8 +221,25 @@ SecPwd::SecPwd(const char *dir, const char* fileName,
 if(stat(certDir,&st) != 0)
 throw SecurityException(DIR_NOTFOUND, (char *)certDir);
 
-certFile = buildName(certDir, fileName, serverName, CER);
-activeCertFile = buildName(certDir, activeFileName, serverName, 
ACTIVE_CER);
+if (lcid == 0x804) // if local charset is not utf8
+{
+serverNameGBKToUtf8 = (char *)malloc(MAX_SQL_IDENTIFIER_LEN + 1);
--- End diff --

You use malloc to allocate this storage but delete to free it. This 
behavior is undefined in general. It would  be better to use free() to delete 
it, or to use new instead of malloc.


---

[GitHub] trafodion pull request #1750: [TRAFODION-3238] ODBC can not connect to serve...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1750#discussion_r237304599
  
--- Diff: win-odbc64/security_dll/native/source/secpwd.cpp ---
@@ -217,8 +221,25 @@ SecPwd::SecPwd(const char *dir, const char* fileName,
 if(stat(certDir,&st) != 0)
 throw SecurityException(DIR_NOTFOUND, (char *)certDir);
 
-certFile = buildName(certDir, fileName, serverName, CER);
-activeCertFile = buildName(certDir, activeFileName, serverName, 
ACTIVE_CER);
+if (lcid == 0x804) // if local charset is not utf8
--- End diff --

Looking at https://msdn.microsoft.com/en-us/library/cc233965.aspx, it 
appears 0x804 = language tag zh-CN, with default sort order. Do we know that 
all other locales use UTF-8? (Perhaps the comment on this line should be, "if 
locale ID is zh-CN")


---

[GitHub] trafodion pull request #1748: [TRAFODION-3236] fix the issue of buffer overr...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1748#discussion_r236024891
  
--- Diff: core/sqf/src/tm/tm.cpp ---
@@ -2789,7 +2789,7 @@ void tm_process_msg(BMS_SRE *pp_sre)
 {
 short  lv_ret;
 char   la_send_buffer[4096];
-char   la_recv_buffer[sizeof(Tm_Req_Msg_Type)];
+char   la_recv_buffer[pp_sre->sre_reqDataSize];
--- End diff --

It is not obvious to me why this change solves the problem. For example, at 
line 2808 below, we check pp_src->src_reqDataSize to see if it is too big to 
fit in la_recv_buffer, and if so, we dynamically allocate a buffer 
la_recv_buffer_ddl for it. It looks like the remaining code in the method makes 
assumptions that certain message types are always shorter than 
sizeof(Tm_Req_Msg_Type) though; but then the logic would read the message into 
the wrong buffer and it would fail in some other way. Could you provide an 
explanation of why it solves the problem?

Also, I'm wondering if we could get rid of dynamically allocating 
la_recv_buffer_ddl and just use la_recv_buffer, by using dynamic array sizing 
as you have done here. That would simplify the logic and reduce heap pressure.

@zcorrea, what do you think?


---

[GitHub] trafodion pull request #1741: [TRAFODION-3190] expression involving NULL sho...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1741#discussion_r235216010
  
--- Diff: core/sql/exp/exp_bool.cpp ---
@@ -96,7 +96,7 @@ ex_expr::exp_return_type ex_branch_clause::eval(char 
*op_data[],
   switch (getOperType())
 {
 case ITM_AND:
-  if (*(Lng32 *)op_data[1] == 0)
+  if (*(Lng32 *)op_data[1] == 0 || *(Lng32 *)op_data[1] == -1)  // 
null treated as false
--- End diff --

I don't believe this is correct. Consider the query, "select a from t1x 
where not(b = 0 and c = 0)". When B and C are both null, both equal predicates 
evaluate to null, and the AND evaluates to null. The NOT then also evaluates to 
null. The WHERE clause should treat the result of the NOT as false. But with 
this fix, the result of the AND will be false, making the NOT true. There needs 
to be three cases here for ITM_AND: If the first operand is false, then the AND 
is false. If the first operand is true, then the result is the second operand. 
If the first operand is null, then if the second operand is false, the result 
is false otherwise the result is null. Similar logic needs to be added to the 
ITM_OR case.


---

[GitHub] trafodion pull request #1746: [TRAFODION-3234] Add support for hive partitio...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1746#discussion_r235207369
  
--- Diff: core/sql/executor/HiveClient_JNI.cpp ---
@@ -68,6 +68,27 @@ char* HiveClient_JNI::getErrorText(HVC_RetCode errEnum)
 return (char*)hvcErrorEnumStr[errEnum-HVC_FIRST-1];
 }
 

+//
+// 

+//
+HiveClient_JNI* HiveClient_JNI::newInstance(NAHeap *heap, HVC_RetCode 
&retCode)
+{
+   QRLogger::log(CAT_SQL_HDFS, LL_DEBUG, "HiveClient_JNI::newInstance() 
called.");
--- End diff --

Just curious: Why do we use CAT_SQL_HDFS in this QRLogger::log call, but 
elsewhere in this module we use CAT_SQL_HBASE? (Perhaps this use is correct and 
the others are all wrong?)


---

[GitHub] trafodion pull request #1746: [TRAFODION-3234] Add support for hive partitio...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1746#discussion_r235210675
  
--- Diff: core/sql/optimizer/hiveHook.h ---
@@ -140,31 +145,42 @@ struct hive_sd_desc
char* nullFormat_;
 
NABoolean isCompressed_;
+   const char *partitionColValues_;
 
struct hive_sd_desc* next_;
 
-   hive_sd_desc(Int32 sdID, const char* loc, Int64 creationTS, Int32 
buckets,
+   hive_sd_desc(NAHeap *heap, Int32 sdID, const char* loc, Int64 
creationTS, Int32 buckets,
 const char* ift, const char* of, 
 const char* nf,
 char knd,
 struct hive_column_desc* column,
 struct hive_skey_desc* skey,
 struct hive_bkey_desc* bkey,
 char fieldTerminator, char recordTerminator,
-const NABoolean isCompressed
+const NABoolean isCompressed,
+const char *pColVals
 )
-: sdID_(sdID), buckets_(buckets), kind_(knd), column_(column),
-  skey_(skey), bkey_(bkey), 
-  fieldTerminator_(fieldTerminator),
-  recordTerminator_(recordTerminator),
-  isCompressed_(isCompressed),
-  next_(NULL)
-  {
-location_ = strduph(loc, CmpCommon::contextHeap());
-inputFormat_ = strduph(ift, CmpCommon::contextHeap()); 
-outputFormat_= strduph(of, CmpCommon::contextHeap());
-nullFormat_ = (nf ? strduph(nf, CmpCommon::contextHeap()) : NULL);
-  }
+
+: heap_(heap), sdID_(sdID), creationTS_(creationTS),
+  buckets_(buckets), kind_(knd), column_(column),
+  skey_(skey), bkey_(bkey), 
+  fieldTerminator_(fieldTerminator),
+  recordTerminator_(recordTerminator),
+  isCompressed_(isCompressed),
+  next_(NULL)
+   {
+ if (loc != NULL)
+location_ = strduph(loc, heap_);
+ else
+location_ = NULL;
+ inputFormat_ = strduph(ift, heap_);
+ outputFormat_= strduph(of, heap_);
+ nullFormat_ = (nf ? strduph(nf, heap_) : NULL);
+ if (pColVals)
+   partitionColValues_ = strduph(pColVals, heap_);
+ else
+   partitionColValues_ = NULL;
--- End diff --

The code is correct. It's interesting that the styles are mixed though: For 
nf, we use a ternary operator and for loc and pColVals we use if/then/else.


---

[GitHub] trafodion pull request #1746: [TRAFODION-3234] Add support for hive partitio...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1746#discussion_r235206682
  
--- Diff: core/sql/executor/HiveClient_JNI.cpp ---
@@ -42,17 +43,16 @@ static const char* const hvcErrorEnumStr[] =
  ,"Java exception in close()."
  ,"Preparing parameters for exists()."
  ,"Java exception in exists()."
- ,"Preparing parameters for getHiveTableStr()."
- ,"Java exception in getHiveTableStr()."
- ,"Preparing parameters for getHiveTableParameters()."
- ,"Java exception in getHiveTableParameters()."
  ,"Preparing parameters for getRedefTime()."
  ,"Java exception in getRedefTime()."
  ,"Java exception in getAllSchemas()."
  ,"Preparing parameters for getAllTables()."
  ,"Java exception in getAllTables()."
  ,"Preparing parameters for executeHiveSQL()."
  ,"Java exception in executeHiveSQL()."
+ ,"Preparing parameters for getHiveTableInfo()."
+ ,"Java exception in getHiveTableInfo()."
+ ,"Error in getHiveTableInfoDetails()."
--- End diff --

Should there also be error text for HVC_ERROR_POPULATE_SDS_ERROR? More 
generally, the entries in this array of strings do not match in number those of 
the enum HVC_RetCode in HiveClient_JNI.h.


---

[GitHub] trafodion pull request #1746: [TRAFODION-3234] Add support for hive partitio...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1746#discussion_r235211297
  
--- Diff: core/sql/sqlcomp/CmpDescribe.cpp ---
@@ -2355,10 +2355,134 @@ short CmpDescribeHiveTable (
   NAString colString(buf);
   Int32 j = i;
   outputColumnLine(space, colString, j);
-}
+  }
 
   outputShortLine(space, "  )");
 
+  // show hive table partitions and buckets defination.
+  // this default schema name is what the Hive default schema is called in 
SeaHive
--- End diff --

What's SeaHive?


---

[GitHub] trafodion pull request #1746: [TRAFODION-3234] Add support for hive partitio...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1746#discussion_r235211551
  
--- Diff: core/sql/sqlcomp/CmpDescribe.cpp ---
@@ -2355,10 +2355,134 @@ short CmpDescribeHiveTable (
   NAString colString(buf);
   Int32 j = i;
   outputColumnLine(space, colString, j);
-}
+  }
 
   outputShortLine(space, "  )");
 
+  // show hive table partitions and buckets defination.
+  // this default schema name is what the Hive default schema is called in 
SeaHive
+  HiveMetaData* md = bindWA.getSchemaDB()->getNATableDB()->getHiveMetaDB();
+  NAString defSchema = 
ActiveSchemaDB()->getDefaults().getValue(HIVE_DEFAULT_SCHEMA);
+  defSchema.toUpper();
+  struct hive_tbl_desc* htbl;
--- End diff --

This variable doesn't seem to be used anywhere. Can we delete it?


---

[GitHub] trafodion pull request #1746: [TRAFODION-3234] Add support for hive partitio...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1746#discussion_r235206424
  
--- Diff: core/sql/executor/HiveClient_JNI.cpp ---
@@ -42,17 +43,16 @@ static const char* const hvcErrorEnumStr[] =
  ,"Java exception in close()."
--- End diff --

Should there also be exception text for ,HVC_ERROR_INIT_EXCEPTION?


---

[GitHub] trafodion pull request #1746: [TRAFODION-3234] Add support for hive partitio...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1746#discussion_r235211107
  
--- Diff: core/sql/sqlcomp/CmpDescribe.cpp ---
@@ -2355,10 +2355,134 @@ short CmpDescribeHiveTable (
   NAString colString(buf);
   Int32 j = i;
   outputColumnLine(space, colString, j);
-}
+  }
 
   outputShortLine(space, "  )");
 
+  // show hive table partitions and buckets defination.
--- End diff --

Nit. Typo: defination (should be "definition")


---

[GitHub] trafodion pull request #1747: [TRAFODION-3235] add div and trim function for...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1747#discussion_r234716204
  
--- Diff: core/conn/odb/src/odb.c ---
@@ -10076,6 +10114,7 @@ static int Oloadbuff(int eid)
 clock_gettime(CLOCK_MONOTONIC, &tsp1);
 #endif
 Or = SQLExecute(thps[tid].Os) ; /* Execute INSERT 
(load/copy) or tgt command */
+SQLLEN tLastRow = -1;   /* this is special solution for 
China Union Pay, print only first error message for state 22003 */
--- End diff --

Probably best not to put customer names into the code. Consider changing 
the comment to just "/* print only first error message for state 22003 */"


---

[GitHub] trafodion pull request #1747: [TRAFODION-3235] add div and trim function for...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1747#discussion_r234715228
  
--- Diff: core/conn/odb/src/odb.c ---
@@ -6246,7 +6248,8 @@ static void Oload(int eid)
  trt[16];   /* translit to array */ 
 char op;/* 1=substr, 2=dconv, 3=tconv, 4=tsconv, 
5=replace,
6=toupper, 7=tolower, 8=firstup, 
9=csubstr, 10=translit,
-   11=comp, 12=comp3, 13=zoned, 
14=emptyasconst, 15=emptyasempty */
+   11=comp, 12=comp3, 13=zoned, 
14=emptyasconst, 15=emptyasempty,
+   16=div */
--- End diff --

Should the comments also include 17=trim?


---

[GitHub] trafodion pull request #1747: [TRAFODION-3235] add div and trim function for...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1747#discussion_r234717573
  
--- Diff: core/conn/odb/src/odb.c ---
@@ -14685,6 +14743,56 @@ static void addGlobalPointer(void *ptr)
 globalPointers[nGlobalPointers++] = ptr;
 }
 
+/* is_valid_numeric:
+*  check if the string is valid numeric
+*
+*  Input: str: string to be validate.
+*  Input: n: length of str
+*
+*  return: if str is valid numeric return 1 else return 0
+*/
+static int is_valid_numeric(const char* str, size_t n) {
+int s = 1;
+for (size_t i = 0; i < n; ++i) {
+switch (s) {
+case 1:
+if (str[i] == '+' || str[i] == '-') s = 2;
+else if (isdigit(str[i])) s = 3;
+else return 0;
+break;
+case 2:
+if (!isdigit(str[i])) return 0;
+s = 3;
+break;
+case 3:
+if (str[i] == '.') s = 4;
+else if (str[i] == 'e') s = 6;
+else if (!isdigit(str[i])) return 0;
+break;
+case 4:
+if (!isdigit(str[i])) return 0;
+s = 5;
+break;
+case 5:
+if (str[i] == 'e') s = 6;
+else if (!isdigit(str[i])) return 0;
+break;
+case 6:
+if (str[i] == '+' || str[i] == '-') s = 7;
+else if (isdigit(str[i])) s = 7;
+else return 0;
+break;
+case 7:
+if (!isdigit(str[i])) return 0;
+break;
+default:
+return 0;
+}
+}
+if (s == 3 || s == 7 || s == 5) return 1;
--- End diff --

I am wondering if s == 4 should be allowed here too. Consider the string 
"1234."


---

[GitHub] trafodion pull request #1739: [TRAFODION-3230] The number of error message i...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1739#discussion_r231655749
  
--- Diff: core/conn/odb/src/odb.c ---
@@ -10183,6 +10183,7 @@ static int Oloadbuff(int eid)
 }
 Oi++;
 }
+Oi = 1; //the record number need to be initialize for next loop
--- End diff --

This is fine, but it might be a bit clearer to the reader if the 
initialization was at the beginning of the loop, for example at line 10090.


---

[GitHub] trafodion pull request #1728: [TRAFODION-2626] Make logs directory location ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1728#discussion_r226129024
  
--- Diff: core/sql/sqludr/SqlUdrPredefLogReader.cpp ---
@@ -530,37 +530,28 @@ void 
ReadCppEventsUDFInterface::processData(UDRInvocationInfo &info,
   {
 char* logrootdir = NULL;
 char* confrootdir = NULL;
+
+logrootdir = getenv("TRAF_LOG");
+if (strlen(logrootdir) > 1000)
+   throw UDRException(38001, "TRAF_HOME is longer than 1000 characters");
+std::string logDirName(logrootdir);
+
 switch (logLocationIndex) 
 {
-case 0: // sqroot, for all logs other than dcs
-  logrootdir = getenv("TRAF_HOME");
-  if (strlen(logrootdir) > 1000)
-   throw UDRException(38001, "TRAF_HOME is longer than 1000 characters");
+case 0: // no sub-directory
   break ;
 case 1:
-  logrootdir = getenv("DCS_INSTALL_DIR");
-  if (!logrootdir)
-   throw UDRException(38001, "DCS_INSTALL_DIR not set");
-  else if (strlen(logrootdir) > 1000)
-   throw UDRException(38001, "DCS_INSTALL_DIR is longer than 1000 
characters");
+  logDirName += "/dcs";
   break ;
 case 2:
-  logrootdir = getenv("REST_INSTALL_DIR");
-  if (!logrootdir)
-   throw UDRException(38001, "REST_INSTALL_DIR not set");
-  else if (strlen(logrootdir) > 1000)
-   throw UDRException(38001, "REST_INSTALL_DIR is longer than 1000 
characters");
-  break ;
+  logDirName += "/rest";
--- End diff --

You need a "break;" statement after this one, otherwise you fall through to 
the default case and raise an exception. This is probably why test udr/TEST002 
failed in the regression test run.


---

[GitHub] trafodion pull request #1728: [TRAFODION-2626] Make logs directory location ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1728#discussion_r226128603
  
--- Diff: core/sql/sqludr/SqlUdrPredefLogReader.cpp ---
@@ -530,37 +530,28 @@ void 
ReadCppEventsUDFInterface::processData(UDRInvocationInfo &info,
   {
 char* logrootdir = NULL;
 char* confrootdir = NULL;
+
+logrootdir = getenv("TRAF_LOG");
+if (strlen(logrootdir) > 1000)
+   throw UDRException(38001, "TRAF_HOME is longer than 1000 characters");
--- End diff --

Should say, TRAF_LOG is longer than 1000 characters


---

[GitHub] trafodion pull request #1730: [TRAFODION-3223] Don't scale down for non-Puts...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1730

[TRAFODION-3223] Don't scale down for non-Puts when estimating row counts

The estimateRowCount code in HBaseClient.java tried to scale down row 
counts by the proportion of non-Put cells in the file. That is, it was trying 
to estimate row count from cell count, in part by discounting the effect of 
Delete tombstone cells. It was doing this on the basis of a sample of 500 rows 
in one HFile.

We find, however, that with time-ordered data that is aged out, the Delete 
cells are not uniformly distributed but instead tend to clump in one place. If 
we are unlucky and get an HFile that begins with 500 Delete tombstones, we will 
incorrectly assume most of the table consists of deleted rows and drastically 
underestimate the number of rows.

Drastically underestimating can be very bad. It is much better to 
overestimate. So the code that attempted to scale down row count based on the 
number of non-Put cells has been deleted. Also, if we find that the number of 
Puts in our sample is very small (< 50), we will instead ignore the sample and 
use the total number of entries.

The changes described above are in HBaseClient.java.

There are two other small, unrelated changes in this pull request as well:

1. The regression test filter for filtering out SYSKEYS has been changed. 
The current minimum number of decimal digits in a SYSKEY is 15; the filter was 
assuming they were at least 16 digits. This would lead to regression failures 
if someone was very unlucky and got just the wrong Linux thread ID for their 
process.

2. An uninitialized member of class ExRtFragTable is now initialized. This 
is a long-standing bug; the changes for pull request 
https://github.com/apache/trafodion/pull/1724 made it observable. For random 
parallel queries, the Executor GUI might come up at run time if the 
uninitialized value happened to be non-zero.

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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3223

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

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


commit 898812f84c510ab8798d5af6e3e63559f4078a07
Author: Dave Birdsall 
Date:   2018-10-17T22:06:44Z

[TRAFODION-3223] Don't scale down for non-Puts when estimating row counts




---

[GitHub] trafodion pull request #1723: TRAFODION - 3218 User still has privilege afte...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1723#discussion_r225695077
  
--- Diff: core/sql/optimizer/NATable.cpp ---
@@ -6805,46 +6809,94 @@ void NATable::getPrivileges(TrafDesc * priv_desc)
   ComSecurityKeySet secKeyVec(heap_);
   if (priv_desc == NULL)
   {
-if (isHiveTable() || isHbaseCellTable() ||
-isHbaseRowTable() || isHbaseMapTable())
+if (!isSeabaseTable())
--- End diff --

The old code would return if this "if" were true, but the new code does not 
(because the return statement after the "else" was placed inside braces. Was 
this intentional?


---

[GitHub] trafodion pull request #1723: TRAFODION - 3218 User still has privilege afte...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1723#discussion_r225662018
  
--- Diff: core/sql/executor/ExExeUtilGet.cpp ---
@@ -1665,10 +1665,10 @@ NABoolean 
ExExeUtilGetMetadataInfoTcb::checkUserPrivs(
   if (ComUser::isRootUserID())
 return FALSE;
 
-  // any user granted the DB__ROOTROLE sees everything
   Int32 numRoles;
   Int32 *roleList;
-  if (currContext->getRoleList(numRoles, roleList) == SUCCESS)
+  Int32 *granteeList;
--- End diff --

Who deletes the memory for the granteeList? (or the roleList for that 
matter)


---

[GitHub] trafodion pull request #1723: TRAFODION - 3218 User still has privilege afte...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1723#discussion_r225659935
  
--- Diff: core/sql/common/ComSecurityKey.cpp ---
@@ -191,33 +216,41 @@ bool buildSecurityKeys( const int32_t userID,
   return false;
   }
 
-  // If the grantee is a role, generate a special security key
-  // If the role is revoked from the user, this key takes affect
+  // If the grantee is a role, generate special security keys, one for
+  // the user and one for each of the user's roles. 
+  // If the role is revoked from the user these key takes affect
   if (PrivMgr::isRoleID(granteeID))
   {
-ComSecurityKey key (userID, granteeID, 
ComSecurityKey::SUBJECT_IS_USER);
-if (doDebug)
+char buf [200];
--- End diff --

This variable isn't used anywhere. Maybe we should delete it?


---

[GitHub] trafodion pull request #1723: TRAFODION - 3218 User still has privilege afte...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1723#discussion_r225658074
  
--- Diff: core/sql/cli/Context.cpp ---
@@ -4199,19 +4219,35 @@ RETCODE ContextCli::setDatabaseUserByName(const 
char *userName)
 // *
 // * Function: ContextCli::getRoleList
 // *
-// * Return the role IDs granted to the current user 
+// * Return the role IDs and their grantees for the current user.  
 // *   If the list of roles is already stored, just return this list.
 // *   If the list of roles does not exist extract the roles granted to the
 // * current user from the Metadata and store in roleIDs_.
 // *
 // 

 RETCODE ContextCli::getRoleList(
-  Int32 &numRoles,
-  Int32 *&roleIDs)
+  Int32 &numEntries,
+  Int32 *& roleIDs,
+  Int32 *& granteeIDs)
 {
   // If role list has not been created, go read metadata
   if (roleIDs_ == NULL)
   {
+// If authorization is not enabled, just setup the PUBLIC role
+CmpContext *cmpCntxt = CmpCommon::context();
+ex_assert(cmpCntxt, "No compiler context exists");
--- End diff --

The assert is fine, but unnecessary since we'd just core anyway on the very 
next statement when we dereference a null pointer.


---

[GitHub] trafodion pull request #1718: [TRAFODION-3214] ODBC error message is truncat...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1718#discussion_r220703185
  
--- Diff: win-odbc64/odbcclient/drvr35/cdiag.cpp ---
@@ -378,15 +378,15 @@ SQLRETURN CDiagRec::GetDiagRec(SQLSMALLINT
RecNumber,
 
//Double strLen to circumvent the bug in driver manager, that 
requires us to give 
//the NO. OF BYTES instead of no. of characters
-   strLen = tmpStrLen * 2;
+   strLen = tmpStrLen;
--- End diff --

Should the comment before this statement be deleted?


---

[GitHub] trafodion pull request #1698: [TRAFODION-2968] update SQL manual for MySQL f...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1698#discussion_r213061994
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -11290,6 +11295,99 @@ ID
 --- 1 row(s) selected.
 ```
 
+<<<
+[[uuid_function]]
+== UUID Function
+Returns a Universal Unique Identifier (UUID) generated according to RFC 
4122. 
+A UUID is designed as a number that is globally unique in space and time. 
+Two calls to UUID() are expected to generate two different values, even if 
+these calls are performed on two separate devices not connected to each 
other.
+
+NOTE: But UUID() can only be used in SELECT list, or very simple 
expressions to get a UUID.
--- End diff --

OK, thanks.


---

[GitHub] trafodion pull request #1698: [TRAFODION-2968] update SQL manual for MySQL f...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1698#discussion_r213061831
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -442,7 +444,10 @@ the expressions have NULL values, the function returns 
a NULL value.
 | <>   | 
Returns the value of the first operand unless it is NULL, in which case it 
returns the value of the second operand.
 | <> | 
Returns either the database user name of the current user who invoked the 
function or the database user name
 associated with the specified user ID number.
-| <> | 
Returns a globally unique identifier.
+| <> | 
Returns a globally unique identifier. (Oracle extention)
+| <> | 
Returns a globally unique identifier.
--- End diff --

That's fine. 


---

[GitHub] trafodion pull request #1698: [TRAFODION-2968] update SQL manual for MySQL f...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1698#discussion_r213061940
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -11290,6 +11295,99 @@ ID
 --- 1 row(s) selected.
 ```
 
+<<<
--- End diff --

OK, thanks.


---

[GitHub] trafodion pull request #1704: [TRAFODION-2952] large amount of data will cau...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1704#discussion_r213058744
  
--- Diff: core/sql/cli/Cli.cpp ---
@@ -10599,7 +10599,10 @@ static Lng32 SeqGenCliInterfaceUpdAndValidateMulti(
   
   numTries++;
   
-  DELAY(100 + numTries*25);
+  if( 100 + numTries*25 < 1000)   //MAX is 1 second
+  DELAY(100 + numTries*25);
+  else
+  DELAY(1000);
--- End diff --

You might want to consider adding some small random amount to this maximum 
delay. Otherwise you might get a bunch of processes in lock step, all retrying 
at the same time.


---

[GitHub] trafodion pull request #1704: [TRAFODION-2952] large amount of data will cau...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1704#discussion_r213058185
  
--- Diff: core/sql/sqlcomp/DefaultConstants.h ---
@@ -2946,6 +2946,12 @@ enum DefaultConstants
   // this is used to change cache size of sequence numbers for a session.
   // It overwrites the cache size that was specified during sequence 
creation.
   TRAF_SEQUENCE_CACHE_SIZE,
+ 
+  // this is used to set the retry time if two concurrent update of 
sequence
+  // conflict, and how many times will retry
+  // by default it is 250, when you saw error 1583, you can try to increase
--- End diff --

In nadefaults.cpp, it looks like the default is 100, not 250, so the 
comment is wrong.


---

[GitHub] trafodion pull request #1698: [TRAFODION-2968] update SQL manual for MySQL f...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1698#discussion_r212092534
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -11290,6 +11295,99 @@ ID
 --- 1 row(s) selected.
 ```
 
+<<<
--- End diff --

Should there be a SYS_GUID example somewhere also?


---

[GitHub] trafodion pull request #1698: [TRAFODION-2968] update SQL manual for MySQL f...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1698#discussion_r212092254
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -11290,6 +11295,99 @@ ID
 --- 1 row(s) selected.
 ```
 
+<<<
+[[uuid_function]]
+== UUID Function
+Returns a Universal Unique Identifier (UUID) generated according to RFC 
4122. 
+A UUID is designed as a number that is globally unique in space and time. 
+Two calls to UUID() are expected to generate two different values, even if 
+these calls are performed on two separate devices not connected to each 
other.
+
+NOTE: But UUID() can only be used in SELECT list, or very simple 
expressions to get a UUID.
--- End diff --

Do we prevent it from being used in the WHERE clause? If not, suggested 
word-smith: "NOTE: UUID() behavior is non-deterministic. Therefore it is best 
to use it only in SELECT lists. Using it in a WHERE clause, for example, may 
result in non-deterministic predicate evaluation."


---

[GitHub] trafodion pull request #1698: [TRAFODION-2968] update SQL manual for MySQL f...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1698#discussion_r212091583
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -442,7 +444,10 @@ the expressions have NULL values, the function returns 
a NULL value.
 | <>   | 
Returns the value of the first operand unless it is NULL, in which case it 
returns the value of the second operand.
 | <> | 
Returns either the database user name of the current user who invoked the 
function or the database user name
 associated with the specified user ID number.
-| <> | 
Returns a globally unique identifier.
+| <> | 
Returns a globally unique identifier. (Oracle extention)
+| <> | 
Returns a globally unique identifier.
--- End diff --

How does this contrast to SYS_GUID? Is UUID standard? Or is it like some 
other vendor?


---

[GitHub] trafodion pull request #1698: [TRAFODION-2968] update SQL manual for MySQL f...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1698#discussion_r212091436
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -442,7 +444,10 @@ the expressions have NULL values, the function returns 
a NULL value.
 | <>   | 
Returns the value of the first operand unless it is NULL, in which case it 
returns the value of the second operand.
 | <> | 
Returns either the database user name of the current user who invoked the 
function or the database user name
 associated with the specified user ID number.
-| <> | 
Returns a globally unique identifier.
+| <> | 
Returns a globally unique identifier. (Oracle extention)
--- End diff --

Typo: extention should be extension. Suggested wordsmith: "Returns a 
globally unique identifier, similar to Oracle's SYS_GUID function."


---

[GitHub] trafodion pull request #1699: [TRAFODION-2404] enhance the regexpr performan...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1699#discussion_r211423746
  
--- Diff: core/sql/exp/exp_like.cpp ---
@@ -513,11 +512,16 @@ ex_expr::exp_return_type 
ExRegexpClauseChar::eval(char *op_data[],
 
   str_cpy_all(pattern, op_data[2], len2);
   str_cpy_all(srcStr, op_data[1], len1);
-
-  z = regcomp(®, pattern, cflags);
+  if(rpattern_ != pattern)
--- End diff --

This is a pointer comparison rather than a content comparison. Since 
rpattern_ is a data member and pattern is a stack variable, I expect this 
comparison will always mismatch, so (rpattern_ != pattern) will always be true. 
I am guessing that you intended to compare the content instead?


---

[GitHub] trafodion pull request #1695: [TRAFODION-3184] Correct Syntax Descriptions a...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1695#discussion_r210705551
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_language_elements.adoc ---
@@ -776,14 +776,105 @@ of digits in the fractional seconds and is stored in 
four bytes. The
 default for _time-precision_ is 0, and the maximum is 6.
 
 * `TIMESTAMP [(_timestamp-precision_)]`
+
++
+** specifies a datetime column that, without the optional
--- End diff --

The wording of this paragraph is a bit awkward. Possible wordsmith: "** 
specifies a datetime column. If `_timestamp-precision_` is specified and is 
zero, the timestamp in external form is `-mm-dd hh:mm:ss` and the database 
storage is 7 bytes. In all other cases, the timestamp in external form is 
`-mm-dd hh:mm:ss.ff`, the number of digits to the right of the decimal 
point being equal to `_timestamp-precision_`. If `_timestamp-precision_` is 
omitted, it defaults to six. Database storage is 11 bytes in these cases.


---

[GitHub] trafodion pull request #1695: [TRAFODION-3184] Correct Syntax Descriptions a...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1695#discussion_r210709602
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_language_elements.adoc ---
@@ -776,14 +776,105 @@ of digits in the fractional seconds and is stored in 
four bytes. The
 default for _time-precision_ is 0, and the maximum is 6.
 
 * `TIMESTAMP [(_timestamp-precision_)]`
+
++
+** specifies a datetime column that, without the optional
--- End diff --

Just noticed that you have a table below that gives the storage sizes. So 
we don't even have to mention it in this paragraph. Possible revised wordsmith: 
"** specifies a datetime column. If _timestamp-precision_ is specified and is 
zero, the timestamp in external form is -mm-dd hh:mm:ss. In all other 
cases, the timestamp in external form is -mm-dd hh:mm:ss.ff, the number 
of digits to the right of the decimal point being equal to 
_timestamp-precision_. If _timestamp-precision_ is omitted, it defaults to six."


---

[GitHub] trafodion pull request #1682: [TRAFODION-3177] Error when selecting count(*)...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1682#discussion_r210397770
  
--- Diff: core/sql/sqludr/SqlUdrPredefLogReader.cpp ---
@@ -1311,17 +1362,19 @@ bool ReadCppEventsUDFInterface::validateEvent(const 
UDRInvocationInfo &info,
 // All other comparisons are assumed to be string compares
 else
 {
-  // convert predicate value
+  // convert and trim predicate value
   temp = constStr;
   constStr.clear();
   for(size_t j = 0; j < temp.size(); ++j)
 constStr += (std::toupper(temp[j]));
+  constStr.erase(constStr.find_last_not_of(" ")+1);
--- End diff --

What happens when find_last_not_of returns string::npos? (that is, when 
there are no trailing blanks?) It seems string::npos is defined as -1 
(unsigned), so adding 1 to it would get zero and we'd do erase(0) which erases 
the whole string?


---

[GitHub] trafodion pull request #1684: [TRAFODION-3179] Correct Syntax Descriptions o...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1684#discussion_r208400577
  
--- Diff: docs/spj_guide/src/asciidoc/_chapters/develop_spjs.adoc ---
@@ -83,19 +83,19 @@ PIC[TURE] X^1^ +
 NCHAR +
 NCHAR VARYING +
 NATIONAL CHAR[ACTER] +
-NATIONAL CHAR[ACTER] VARYING | java.lang.string
-| DATE | java.sql.date
-| TIME | java.sql.time
-| TIMESTAMP | java.sql.timestamp
+NATIONAL CHAR[ACTER] VARYING | java.lang.String
+| DATE | java.sql.Date
+| TIME | java.sql.Time
+| TIMESTAMP | java.sql.Timestamp
 | DEC[IMAL]^2^ +
 PIC[TURE] S9^3^ +
-NUMERIC (including numeric with a precision greater than eighteen)^2^ | 
java.math.bigdecimal
+NUMERIC (including numeric with a precision greater than eighteen)^2^ | 
java.math.Bigdecimal
--- End diff --

Oops... spotted a typo. It should be "java.math.BigDecimal" (note the 
capital D in the middle of things)


---

[GitHub] trafodion pull request #1683: [TRAFODION-3178] Correct Syntax Descriptions o...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1683#discussion_r208396395
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_clauses.adoc ---
@@ -1053,8 +1053,7 @@ expressions in each item must be the same as the 
number of value columns
 in the column list.
 +
 In the example TRANSPOSE A,B,C AS V, the items are A,B, and C, and the
-value column is V. This form can be thought of as a shorter way of writing 
TRANSPOSE
-(A),(B),(C) AS (V).
+value column is V. This form can be thought of as a shorter way of writing 
TRANSPOSE (A),(B),\(C) AS (V).
--- End diff --

Not sure why the slash is before "(C)"; I don't think it is correct. Or is 
the slash an escape? Maybe "(C)" is interpreted as the copyright symbol by the 
rendering software?


---

[GitHub] trafodion pull request #1683: [TRAFODION-3178] Correct Syntax Descriptions o...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1683#discussion_r208397031
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -31,20 +31,27 @@ and expressions that you can use in {project-name} SQL 
statements. The
 functions and expressions are categorized according to their
 functionality.
 
-[[standard_normalization]]
-== Standard Normalization
+[[categories]]
+== Categories
 
-For datetime functions, the definition of standard normalization is: If
-the ending day of the resulting date is invalid, the day will be rounded
-DOWN to the last day of the result month.
+Use these types of functions within an SQL value expression:
 
-== Aggregate (Set) Functions
+** Aggregate (Set) Functions
--- End diff --

Do you want to include LOB Functions and Encryption Functions in this list?


---

[GitHub] trafodion pull request #1679: [TRAFODION-3163] Add script to do STFS health ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1679#discussion_r208041994
  
--- Diff: core/sqf/src/tm/tools/dtmci.cpp ---
@@ -1801,6 +1801,20 @@ int main(int argc, char *argv[])
 << "' for status command." << endl;
 }
 }
+else if (!strcmp(lp_nextcmd, "showenv"))
+{
+   get_cmd(lp_inputstr, lp_nextcmd);
+   if (lp_nextcmd[0] == '\0')
+ cout <<"TODO: show all env var\n" << endl;
+   else
+   {
+  const char* v = ms_getenv_str((const char*)lp_nextcmd);
+  if(v == NULL)
+cout <<"*** Enviorement Var not exist" << endl;
--- End diff --

Typo. "Environment"


---

[GitHub] trafodion pull request #1677: TRAFODION-3146 Support ANSI OVERLAY function

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1677#discussion_r207411473
  
--- Diff: core/sql/exp/exp_function.cpp ---
@@ -786,13 +786,18 @@ ExpRaiseErrorFunction::ExpRaiseErrorFunction 
(Attributes **attr,
  NABoolean raiseError,
  const char *constraintName,
  const char *tableName,
- const NABoolean hasStringExp) 
 // -- Triggers
+  const NABoolean 
hasStringExp,  // -- Triggers
+  const char * optionalStr)
 : ex_function_clause (ITM_RAISE_ERROR, (hasStringExp ? 2 : 1), attr, 
space),
-theSQLCODE_(sqlCode),
-constraintName_((char *)constraintName),
-tableName_((char *)tableName)
+  theSQLCODE_(sqlCode),
+  constraintName_((char *)constraintName),
+  tableName_((char *)tableName)
 {
-setRaiseError(raiseError);
+  setRaiseError(raiseError);
+
+
+  strncpy(optionalStr_, optionalStr, MAX_OPTIONAL_STR_LEN);
--- End diff --

This will abend if "optionalStr" is null


---

[GitHub] trafodion pull request #1677: TRAFODION-3146 Support ANSI OVERLAY function

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1677#discussion_r207412672
  
--- Diff: core/sql/bin/SqlciErrors.txt ---
@@ -1535,6 +1535,7 @@ $1~String1 
 8142 Z 9 ADVANCED INFRM LOGONLY An error was artificially 
injected, to test error handling. Testpoint $0~string0, Value $1~string1.
 8143 Z 9 ADVANCED INFRM LOGONLY The requested operation stopped 
$0~Int0 server processes.
 8144 Z 9 ADVANCED MAJOR DIALOUT Corruption is detected in table 
$0~string0$1~string1.
+8145 Z 9 BEGINNER MAJOR DBADMIN This statement is not supported or 
incorrect options were specified. Reason: $0~string0
--- End diff --

Could you add this to the Messages Guide too?


---

[GitHub] trafodion pull request #1658: [TRAFODION-3155] implemented the OVERLAPS pred...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1658#discussion_r206907388
  
--- Diff: core/sql/generator/GenPreCode.cpp ---
@@ -7518,6 +7518,53 @@ ItemExpr * AggrMinMax::preCodeGen(Generator * 
generator)
   return this;
 } // AggrMinMax::preCodeGen()
 
+ItemExpr *Overlaps::preCodeGen(Generator *generator)
+{
+  if (nodeIsPreCodeGenned())
+return getReplacementExpr();
+
+  for (Int32 i = 0; i < getArity(); ++i)
+  {
+if (child(i)) 
+{
+  const NAType &type = 
+child(i)->getValueId().getType();
+  const DatetimeType *operand = (DatetimeType *)&type;
+
+  if (type.getTypeQualifier() == NA_DATETIME_TYPE
+&& (operand->getPrecision() == SQLDTCODE_DATE))
+  {
+child(i) = new (generator->wHeap()) 
+  Cast(child(i), new (generator->wHeap()) 
+  SQLTimestamp(generator->wHeap(), TRUE));
+
+child(i)->bindNode(generator->getBindWA());
+  }
+
+}
+  }
+
+  ItemExpr *newExpr = 
+generator->getExpGenerator()->createExprTree(
--- End diff --

I agree with @zellerh on the interpretation of the standard.


---

[GitHub] trafodion pull request #1658: [TRAFODION-3155] implemented the OVERLAPS pred...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1658#discussion_r206726135
  
--- Diff: core/sql/generator/GenPreCode.cpp ---
@@ -7518,6 +7518,53 @@ ItemExpr * AggrMinMax::preCodeGen(Generator * 
generator)
   return this;
 } // AggrMinMax::preCodeGen()
 
+ItemExpr *Overlaps::preCodeGen(Generator *generator)
+{
+  if (nodeIsPreCodeGenned())
+return getReplacementExpr();
+
+  for (Int32 i = 0; i < getArity(); ++i)
+  {
+if (child(i)) 
+{
+  const NAType &type = 
+child(i)->getValueId().getType();
+  const DatetimeType *operand = (DatetimeType *)&type;
+
+  if (type.getTypeQualifier() == NA_DATETIME_TYPE
+&& (operand->getPrecision() == SQLDTCODE_DATE))
+  {
+child(i) = new (generator->wHeap()) 
+  Cast(child(i), new (generator->wHeap()) 
+  SQLTimestamp(generator->wHeap(), TRUE));
+
+child(i)->bindNode(generator->getBindWA());
+  }
+
+}
+  }
+
+  ItemExpr *newExpr = 
+generator->getExpGenerator()->createExprTree(
--- End diff --

Indeed! I see my error. Thanks.


---

[GitHub] trafodion pull request #1658: [TRAFODION-3155] implemented the OVERLAPS pred...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1658#discussion_r206722914
  
--- Diff: core/sql/generator/GenPreCode.cpp ---
@@ -7518,6 +7518,53 @@ ItemExpr * AggrMinMax::preCodeGen(Generator * 
generator)
   return this;
 } // AggrMinMax::preCodeGen()
 
+ItemExpr *Overlaps::preCodeGen(Generator *generator)
+{
+  if (nodeIsPreCodeGenned())
+return getReplacementExpr();
+
+  for (Int32 i = 0; i < getArity(); ++i)
+  {
+if (child(i)) 
+{
+  const NAType &type = 
+child(i)->getValueId().getType();
+  const DatetimeType *operand = (DatetimeType *)&type;
+
+  if (type.getTypeQualifier() == NA_DATETIME_TYPE
+&& (operand->getPrecision() == SQLDTCODE_DATE))
+  {
+child(i) = new (generator->wHeap()) 
+  Cast(child(i), new (generator->wHeap()) 
+  SQLTimestamp(generator->wHeap(), TRUE));
+
+child(i)->bindNode(generator->getBindWA());
+  }
+
+}
+  }
+
+  ItemExpr *newExpr = 
+generator->getExpGenerator()->createExprTree(
--- End diff --

@zellerh, just wanted to check the analysis. I would have thought 2 > 1 and 
not (2 >=3 and null >= 3) becomes 2 > 1 and not (true and null) which becomes 2 
> 1 and not (null) which becomes 2 > 1 and null which becomes true and null 
which becomes null. So I didn't follow why you thought this evaluates to TRUE. 
What error am I making?


---

[GitHub] trafodion pull request #1669: Fix for TRAFODION-3112

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1669#discussion_r206662600
  
--- Diff: core/sql/regress/compGeneral/EXPECTEDTOK ---
@@ -1,5 +1,5 @@
 sqlparser.y: warning: 61 shift/reduce conflicts [-Wconflicts-sr]
-sqlparser.y: warning: 12 reduce/reduce conflicts [-Wconflicts-rr]
+sqlparser.y: warning: 9 reduce/reduce conflicts [-Wconflicts-rr]
--- End diff --

Cool! Reduced the number of conflicts!


---

[GitHub] trafodion pull request #1669: Fix for TRAFODION-3112

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1669#discussion_r206663223
  
--- Diff: core/sql/bin/SqlciErrors.txt ---
@@ -85,7 +85,7 @@
 1083 Z 9 BEGINNER MAJOR DBADMIN Validation for constraint 
$0~ConstraintName failed; incompatible data exists in table.
 1084 Z 9 BEGINNER MINOR DBADMIN An invalid default value was 
specified for column $0~ColumnName.
 1085 Z 9 BEGINNER MAJOR DBADMIN --- unused ---
-1086 Z 9  U UUU --- unused ---
+1086 Z 9 BEGINNER MAJOR DBADMIN $0~string0 is not a $1~string1.
--- End diff --

Would be good if you could add this new message to the Messages Guide.


---

[GitHub] trafodion pull request #1664: [TRAFODION-2884] Multiple fixes with Name Serv...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1664#discussion_r205629519
  
--- Diff: core/sqf/monitor/linux/process.cxx ---
@@ -4775,46 +4818,85 @@ void CProcessContainer::Bcast (struct message_def 
*msg)
 
 char *CProcessContainer::BuildOurName( int nid, int pid, char *name )
 {
-int i;
-int rem;
-int cnt[4];
-
 const char method_name[] = "CProcessContainer::BuildOurName";
 TRACE_ENTRY;
 
-// Convert Pid into base 35 acsii
-cnt[0] = pid / 42875;
-rem = pid - ( cnt[0] * 42875 );
-cnt[1] = rem / 1225;
-rem -= ( cnt[1] * 1225 );
-cnt[2] = rem / 35;
-rem -= ( cnt[2] * 35 );
-cnt[3] = rem;
+int i;
+int rem;
+int cnt[6];
+
+if (!processNameFormatLong_)
+{
+// Convert Pid into base 35 acsii
+cnt[0] = pid / 42875;// (35 * 35 * 35)
+rem = pid - ( cnt[0] * 42875 );
+cnt[1] = rem / 1225; // (35 * 35)
+rem -= ( cnt[1] * 1225 );
+cnt[2] = rem / 35;
+rem -= ( cnt[2] * 35 );
+cnt[3] = rem;
+
+// Process name format long: '$Zxx' xx = nid,  = pid
 
-// Convert Nid into base 16 acsii
-sprintf(name,"$Z%2.2X",nid);
-for(i=3; i>=0; i--)
-{
-if( cnt[i] < 10 )
-{
-name[i+4] = '0'+cnt[i];
-}
-else
+// Convert Nid into base 16 acsii
+sprintf(name,"$Z%2.2X",nid);
+
+// Convert Pid into base 36 ascii
+for(i=3; i>=0; i--)
 {
-cnt[i] -= 10;
-// we are skipping cap 'o' because it looks like zero.
-if( cnt[i] >= 14 )
+if( cnt[i] < 10 )
 {
-name[i+4] = 'P'+(cnt[i]-14);
+name[i+4] = '0'+cnt[i];
 }
 else
 {
-name[i+4] = 'A'+cnt[i];
+cnt[i] -= 10;
+// we are skipping cap 'o' because it looks like zero.
+if( cnt[i] >= 14 )
+{
+name[i+4] = 'P'+(cnt[i]-14);
+}
+else
+{
+name[i+4] = 'A'+cnt[i];
+}
 }
 }
+name[8] = '\0';
+}
+else
+{
+// We are skipping 'A', 'I', 'O', and 'U' to distinguish between 
zero
+// and one digits, and for political correctness in generated names
--- End diff --

No English four-letter words with just E as the vowel?


---

[GitHub] trafodion pull request #1656: [TRAFODION-3159] Add *Syntax Descriptions and ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1656#discussion_r205178175
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -9066,13 +9066,220 @@ second argument describes the format of the 
character value.
 TO_CHAR(character-expression [,format-string])
 ```
 
+[[to_char_syntax]]
+=== Syntax Description of TO_CHAR
+
 * `_character-expression_`
 +
 is an expression that gives a datetime value.
 
 * `_format-string_`
 +
-is one of the following character string literals:
+.Details of `_format-string_`
+[cols="3*.^"]
+|===
+^| *Supported* `_character-expression_` 
+^| *`_format-string_`* 
+^| *Description* 
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'CC'
+| Two-digit century.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'D'
+| Day of week (1-7), where 1 is Sunday, 6 is Saturday. +
+This is not configurable.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DD'
+| Day of month (01-31).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DDD'
+| Day of year (001-366).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DY'
+a| Name of day, which is a three-letter abbreviation for the day in 
uppercase. +
+
+The following values are returned: +
+
+* MON +
+
+* TUE +
+
+* WED +
+
+* THU +
+
+* FRI +
+
+* SAT +
+
+* SUN
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DAY'
+a| Name of day, which is full uppercase day name padded with blanks to the 
length of 9 characters. +
+
+The following values are returned: +
+
+* MONDAY +
+
+* TUESDAY +
+
+* WEDSDAY +
+
+* THURSDAY +
+
+* FRIDAY +
+
+* SATURDAY +
+
+* SUNDAY
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH'
+| Hour of day in 24-hour format (00-23). +
+'HH' behaves the same as 'HH24'.
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH12'
+| Hour of day in 12-hour format (01-12).
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH24'
+| Hour of day in 24-hour format (00-23). +
+'HH24' behaves the same as 'HH'.
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'J'
+| Julian date (that is, number of days since January 1, 4713 BC).
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'MI'
+| Minute (00-59).
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'MM'
+| Month (01-12).
+
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'MON'
+a| Month, which is a three-letter abbreviation for the month in uppercase. 
+
+
+The following values are returned: +
+
+* JAN +
+
+* FEB +
+
+* MAR +
+
+* APR +
+
+* MAY +
+
+* JUN +
+
+* JUL +
+
+* AUG +
+
+* SEP +
+
+* OCT +
+
+* NOV +
+
+* DEC
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'Q'
+| Quarter (1-4).
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'SS'
+| Second (00-59).
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'W'
+| Week of month.
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'WW'
+a| Week of year. +
+
+* The value 1 is returned for datetime that occurs in the first 7 days of 
the year if the year begins on Sunday. +
--- End diff --

"for a datetime" would be better (here and below)


---

[GitHub] trafodion pull request #1663: [TRAFODION-3152] Fix bug in UPDATE STATISTICS ...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1663

[TRAFODION-3152] Fix bug in UPDATE STATISTICS CREATE SAMPLE

If the "_HIVESTATS_" schema didn't already exist, UPDATE STATISTICS ... 
CREATE SAMPLE and UPDATE STATISTICS ... REMOVE SAMPLE would fail with weird 
errors.

With this change, they now will succeed. The "_HIVESTATS_" schema and 
associated histogram tables will be created automatically.

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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3152

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

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


commit f79b3e5ed5924f78632f3b2815ead1281e77e66a
Author: Dave Birdsall 
Date:   2018-07-24T23:03:20Z

[TRAFODION-3152] Fix bug in UPDATE STATISTICS CREATE SAMPLE




---

[GitHub] trafodion pull request #1653: [TRAFODION-3156] Add *Syntax Descriptions and ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1653#discussion_r204917685
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -4437,35 +4437,670 @@ interval value expression. It returns an exact 
numeric value.
 EXTRACT (datetime-field FROM extract-source)
 ```
 
+[[extract_function_syntax]]
+=== Syntax Descriptions of EXTRACT Function
+
 * `_datetime-field_` is:
 +
-YEAR \| MONTH \| DAY \| HOUR \| MINUTE \| SECOND
+.Details of `_datetime-field_`
+[cols="20%,40%,40%"]
+|===
+^| `_datetime-filed_` 
+^| *Description* 
+^| *Supported* `_extract-source_` 
+
+a| CENTURY 
+| Century.
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+
+| DAY
+a| * `_datetime-expression_` +
+Day.
+* `_interval-expression_` +
+Number of day(s) in the `_interval-expression_`. +
+a| * `_datetime-expression_`
+** DATE 
+** TIMESTAMP 
+
+| DECADE
+a| * `_datetime-expression_` +
+Year field divided by 10.
+* `_interval-expression_` +
+Number of decade(s) in the `_interval-expression_`. +
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+* `_interval-expression_` 
+
+| DOW
+| Day of week (1-7), where 1 is Sunday, 6 is Saturday. +
+This is not configurable.
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| DOY
+| Day of year (1-366).
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+
+| EPOCH
+a| * `_datetime-expression_` +
+Second(s) since 1970-01-01 00:00:00. + 
+The value is negative if `_datetime-expression_` precedes 1970-01-01 
00:00:00.
+* `_interval-expression_` +
+Number of second(s) in the `_interval-expression_`. +
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP
+* `_interval-expression_`
+
+| HOUR
+a| * `_datetime-expression_` +
+Hour (0-23).
+* `_interval-expression_` +
+Number of hour(s) in the `_interval-expression_`. +
+a| * `_datetime-expression_` +
+** TIME
+** TIMESTAMP
+* `_interval-expression_`
+
+| MINUTE
+a| * `_datetime-expression_` +
+Minute (0-59).
+* `_interval-expression_` +
+Number of minute(s) in the `_interval-expression_`. +
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP
+* `_interval-expression_`
+
+| MONTH
+a| * `_datetime-expression_` +
+Month (1-12).
+* `_interval-expression_` +
+Number of month(s) in the `_interval-expression_`. +
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP
+* `_interval-expression_`
+
+| QUARTER
+a| * `_datetime-expression_` +
+Quarter of year (1-4).
+* `_interval-expression_` +
+Number of quarter(s) in the `_interval-expression_`. 
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP
+* `_interval-expression_`
+
+| SECOND
+a| * `_datetime-expression_` +
+Second (0-59).
+* `_interval-expression_` +
+Number of second(s) in the `_interval-expression_`. +
+a| * `_datetime-expression_` +
+** TIME
+** TIMESTAMP
+* `_interval-expression_` +
+
+| WEEK
+a| * `_datetime-expression_` +
+a| Week of year. +
+
+* The value 1 is returned for datetime that occurs in the first 7 days of 
the year if the year begins on Sunday. +
--- End diff --

Minor wordsmith. Should say, "The value 1 is returned for a datetime ... " 
(add an indefinite article before "datetime"). The next few sentences need this 
too.


---

[GitHub] trafodion pull request #1656: [TRAFODION-3159] Add *Syntax Descriptions and ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1656#discussion_r204860494
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -9098,30 +9305,606 @@ is one of the following character string literals:
 ** 'MM/DD/ HH24:MI:SS'
 ** 'DD-MON- HH:MI:SS'
 ** 'MONTH DD, , HH:MI'
-** 'DD.MM. HH24.MI.SS'
-
-Here,  refers to a 4-digit year. YY refers to a two-digit year. MM 
refers to a two-digit month. MON refers to
-a three-letter abbreviation for month ('JAN', 'FEB', 'MAR', 'APR', 'MAY', 
'JUN', 'JUL', 'AUG', 'SEP',
-'OCT', 'NOV' or 'DEC'). MONTH refers to the month spelled out. In the 
output value, the month abbreviation or month
-will appear in upper case.
+** 'DD.MM. HH24.MI.SS' 
 
-HH and HH24 refer to a 2-digit hour field. MI refers to a two-digit 
minutes field. SS refers to a 2-digit seconds field.
++
+Here:
 
-If the _format-string_ argument is omitted, '-MM-DD' is used as the
+***  refers to a four-digit year. 
+*** YY refers to a two-digit year. 
+*** MM refers to a two-digit month. 
+*** MON refers to a three-letter abbreviation for month ('JAN', 'FEB', 
'MAR', 'APR', 'MAY', 'JUN', 'JUL', 'AUG', 'SEP',
+'OCT', 'NOV' or 'DEC'). 
+*** MONTH refers to the month spelled out. In the output value, the month 
abbreviation or month
+will appear in upper case.
+*** HH and HH24 refer to a two-digit hour field. 
+*** MI refers to a two-digit minutes field. 
+*** SS refers to a two-digit seconds field.
+*** If the `_format-string_` argument is omitted, '-MM-DD' is used as 
the
 default for date and timestamp values, and 'HH:MI:SS' is used for time 
values.
 
 [[considerations_for_to_char]]
 === Considerations for TO_CHAR
 
-If the _format-string_ contains only hour, minute or seconds fields, the 
input data type must be time or timestamp.
+If the `_format-string_` contains only hour, minute or seconds fields, the 
input data type must be time or timestamp.
 
-If the _format-string_ contains only year, month or day fields, the input 
data type must be date or timestamp.
+If the `_format-string_` contains only year, month or day fields, the 
input data type must be date or timestamp.
 
-If the _format-string_ contains all fields, and the input data type is 
date, the hour, minute and second fields in the result will be filled with 
zeroes.
+If the `_format-string_` contains all fields, and the input data type is 
date, the hour, minute and second fields in the result will be filled with 
zeroes.
 
 [[examples_of_to_char]]
 === Examples of TO_CHAR
 
+[[examples_of_to_char_cc]]
+ Examples of TO_CHAR (CC)
+
+* This example converts the `_DATE_` value to the character value of 
century.
++
+```
+SQL>SELECT TO_CHAR (DATE '0001-11-02','CC') FROM DUAL;
+
+(EXPR)
+--
+01
+
+--- 1 row(s) selected.
+```
+
+* This example converts the `_TIMESTAMP_` value to the character value of 
century.
++
+```
+SQL>SELECT TO_CHAR (TIMESTAMP '9899-12-31 23:59:59','CC') FROM DUAL;
+
+(EXPR)
+--
+99
+
+--- 1 row(s) selected.
+```
+
+[[examples_of_to_char_d]]
+ Examples of TO_CHAR (D)
+
+* This example converts the `_DATE_` value to the character value of day 
of week.
++
+```
+SQL>SELECT TO_CHAR (DATE '2018-01-01','D') FROM DUAL;
+
+(EXPR)
+--
+2 
+
+--- 1 row(s) selected.
+```
+
+* This example converts the `_TIMESTAMP_` value to the character value of 
day of week.
++
+```
+SQL>SELECT TO_CHAR (TIMESTAMP '2018-04-01 23:59:59','D') FROM DUAL;
+
+(EXPR)
+--
+1 
+
+--- 1 row(s) selected.
+```
+
+[[examples_of_to_char_dd]]
+ Examples of TO_CHAR (DD)
+
+* This example converts the `_DATE_` value to the character value of day 
of month.
++
+```
+SQL>SELECT TO_CHAR (DATE '2018-01-01','DD') FROM DUAL;
+
+(EXPR)
+--
+01
+
+--- 1 row(s) selected. 
+```
+
+* This example converts the `_TIMESTAMP_` value to the character value of 
day of month.
++
+```
+SQL>SELECT TO_CHAR (TIMESTAMP '2018-12-31 23:59:59','DD') FROM DUAL;
+
+(EXPR)
 

[GitHub] trafodion pull request #1656: [TRAFODION-3159] Add *Syntax Descriptions and ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1656#discussion_r204862408
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -9066,13 +9066,220 @@ second argument describes the format of the 
character value.
 TO_CHAR(character-expression [,format-string])
 ```
 
+[[to_char_syntax]]
+=== Syntax Description of TO_CHAR
+
 * `_character-expression_`
 +
 is an expression that gives a datetime value.
 
 * `_format-string_`
 +
-is one of the following character string literals:
+.Details of `_format-string_`
+[cols="3*.^"]
+|===
+^| *Supported* `_character-expression_` 
+^| *`_format-string_`* 
+^| *Description* 
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'CC'
+| Two-digital century.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'D'
+| Day of week (1-7), where 1 is Sunday, 6 is Saturday. +
+This is not configurable.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DD'
+| Day of month (01-31).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DDD'
+| Day of year (001-366).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DY'
+a| Name of day, which is a three-letter abbreviation for the day in 
uppercase. +
+
+Following values are valid: +
+
+* MON +
+
+* TUE +
+
+* WED +
+
+* THU +
+
+* FRI +
+
+* SAT +
+
+* SUN
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DAY'
+a| Name of day, which is full uppercase day name padded with blanks to the 
length of 9 characters. +
+
+Following values are valid: +
+
+* MONDAY +
+
+* TUESDAY +
+
+* WEDSDAY +
+
+* THURSDAY +
+
+* FRIDAY +
+
+* SATURDAY +
+
+* SUNDAY
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH'
+| Hour of day in 24-hour format (00-23). +
+'HH' behaves the same as 'HH24'.
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH12'
+| Hour of day in 12-hour format (01-12).
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH24'
+| Hour of day in 24-hour format (00-23). +
+'HH24' behaves the same as 'HH'.
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'J'
+| Number of days since Julian date (January 1, 4713 BC).
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'MI'
+| Minute (00-59).
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'MM'
+| Month (01-12).
+
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'MON'
+a| Month, which is a three-letter abbreviation for the month in uppercase. 
+
+
+Following values are valid: +
--- End diff --

Suggested wordsmith: "The following values are valid: +"


---

[GitHub] trafodion pull request #1656: [TRAFODION-3159] Add *Syntax Descriptions and ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1656#discussion_r204862138
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -9066,13 +9066,220 @@ second argument describes the format of the 
character value.
 TO_CHAR(character-expression [,format-string])
 ```
 
+[[to_char_syntax]]
+=== Syntax Description of TO_CHAR
+
 * `_character-expression_`
 +
 is an expression that gives a datetime value.
 
 * `_format-string_`
 +
-is one of the following character string literals:
+.Details of `_format-string_`
+[cols="3*.^"]
+|===
+^| *Supported* `_character-expression_` 
+^| *`_format-string_`* 
+^| *Description* 
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'CC'
+| Two-digital century.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'D'
+| Day of week (1-7), where 1 is Sunday, 6 is Saturday. +
+This is not configurable.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DD'
+| Day of month (01-31).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DDD'
+| Day of year (001-366).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DY'
+a| Name of day, which is a three-letter abbreviation for the day in 
uppercase. +
+
+Following values are valid: +
+
+* MON +
+
+* TUE +
+
+* WED +
+
+* THU +
+
+* FRI +
+
+* SAT +
+
+* SUN
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DAY'
+a| Name of day, which is full uppercase day name padded with blanks to the 
length of 9 characters. +
+
+Following values are valid: +
+
+* MONDAY +
+
+* TUESDAY +
+
+* WEDSDAY +
+
+* THURSDAY +
+
+* FRIDAY +
+
+* SATURDAY +
+
+* SUNDAY
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH'
+| Hour of day in 24-hour format (00-23). +
+'HH' behaves the same as 'HH24'.
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH12'
+| Hour of day in 12-hour format (01-12).
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH24'
+| Hour of day in 24-hour format (00-23). +
+'HH24' behaves the same as 'HH'.
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'J'
+| Number of days since Julian date (January 1, 4713 BC).
--- End diff --

Suggested word-smith: "Julian date (that is, number of days since January 
1, 4713 BC)"


---

[GitHub] trafodion pull request #1656: [TRAFODION-3159] Add *Syntax Descriptions and ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1656#discussion_r204862754
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -9066,13 +9066,220 @@ second argument describes the format of the 
character value.
 TO_CHAR(character-expression [,format-string])
 ```
 
+[[to_char_syntax]]
+=== Syntax Description of TO_CHAR
+
 * `_character-expression_`
 +
 is an expression that gives a datetime value.
 
 * `_format-string_`
 +
-is one of the following character string literals:
+.Details of `_format-string_`
+[cols="3*.^"]
+|===
+^| *Supported* `_character-expression_` 
+^| *`_format-string_`* 
+^| *Description* 
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'CC'
+| Two-digital century.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'D'
+| Day of week (1-7), where 1 is Sunday, 6 is Saturday. +
+This is not configurable.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DD'
+| Day of month (01-31).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DDD'
+| Day of year (001-366).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DY'
+a| Name of day, which is a three-letter abbreviation for the day in 
uppercase. +
+
+Following values are valid: +
+
+* MON +
+
+* TUE +
+
+* WED +
+
+* THU +
+
+* FRI +
+
+* SAT +
+
+* SUN
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DAY'
+a| Name of day, which is full uppercase day name padded with blanks to the 
length of 9 characters. +
+
+Following values are valid: +
--- End diff --

Suggest "The following ..."


---

[GitHub] trafodion pull request #1656: [TRAFODION-3159] Add *Syntax Descriptions and ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1656#discussion_r204860831
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -9066,13 +9066,220 @@ second argument describes the format of the 
character value.
 TO_CHAR(character-expression [,format-string])
 ```
 
+[[to_char_syntax]]
+=== Syntax Description of TO_CHAR
+
 * `_character-expression_`
 +
 is an expression that gives a datetime value.
 
 * `_format-string_`
 +
-is one of the following character string literals:
+.Details of `_format-string_`
+[cols="3*.^"]
+|===
+^| *Supported* `_character-expression_` 
+^| *`_format-string_`* 
+^| *Description* 
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'CC'
+| Two-digital century.
--- End diff --

"Two-digit century" would be better.


---

[GitHub] trafodion pull request #1656: [TRAFODION-3159] Add *Syntax Descriptions and ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1656#discussion_r204862548
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -9066,13 +9066,220 @@ second argument describes the format of the 
character value.
 TO_CHAR(character-expression [,format-string])
 ```
 
+[[to_char_syntax]]
+=== Syntax Description of TO_CHAR
+
 * `_character-expression_`
 +
 is an expression that gives a datetime value.
 
 * `_format-string_`
 +
-is one of the following character string literals:
+.Details of `_format-string_`
+[cols="3*.^"]
+|===
+^| *Supported* `_character-expression_` 
+^| *`_format-string_`* 
+^| *Description* 
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'CC'
+| Two-digital century.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'D'
+| Day of week (1-7), where 1 is Sunday, 6 is Saturday. +
+This is not configurable.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DD'
+| Day of month (01-31).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DDD'
+| Day of year (001-366).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DY'
+a| Name of day, which is a three-letter abbreviation for the day in 
uppercase. +
+
+Following values are valid: +
+
+* MON +
+
+* TUE +
+
+* WED +
+
+* THU +
+
+* FRI +
+
+* SAT +
+
+* SUN
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DAY'
+a| Name of day, which is full uppercase day name padded with blanks to the 
length of 9 characters. +
+
+Following values are valid: +
+
+* MONDAY +
+
+* TUESDAY +
+
+* WEDSDAY +
+
+* THURSDAY +
+
+* FRIDAY +
+
+* SATURDAY +
+
+* SUNDAY
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH'
+| Hour of day in 24-hour format (00-23). +
+'HH' behaves the same as 'HH24'.
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH12'
+| Hour of day in 12-hour format (01-12).
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'HH24'
+| Hour of day in 24-hour format (00-23). +
+'HH24' behaves the same as 'HH'.
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'J'
+| Number of days since Julian date (January 1, 4713 BC).
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'MI'
+| Minute (00-59).
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'MM'
+| Month (01-12).
+
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'MON'
+a| Month, which is a three-letter abbreviation for the month in uppercase. 
+
+
+Following values are valid: +
+
+* JAN +
+
+* FEB +
+
+* MAR +
+
+* APR +
+
+* MAY +
+
+* JUN +
+
+* JUL +
+
+* AUG +
+
+* SEP +
+
+* OCT +
+
+* NOV +
+
+* DEC
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'Q'
+| Quarter (1-4).
+
+a| * `_datetime-expression_` 
+** TIME
+** TIMESTAMP 
+| 'SS'
+| Second (00-59).
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'W'
+| Week of month.
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'WW'
+a| Week of year. +
+
+* The value 1 is returned for datetime that occurs in the first 7 days of 
the year if the year begins on Sunday. +
+Otherwise, the value 1 is returned for datetime that occurs in the partial 
week before the start of the first Sunday. 
+
+* The value *53* is returned for datetime that occurs in the last full or 
partial week of the year. 
+
+* The value *54* is returned for datetime that occurs in the last full or 
partial week of the leap year where begins on Saturday.
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'Y'
+| Last digit of year (0-9).
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'YY'
+| Last two digits of year (00-99).
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| 'YYY'
+| Last three digits of year (000-999).
+
+a| * `_datetime-expression_` 
+** DATE
+** TIMESTAMP 
+| ''
+| Four-digit year (0001-).
+|===
+
++
+Besides the `_format-string_` listed above, following character string 
literals are also valid:
--- End diff --

Suggest "... the following ..."


---

[GitHub] trafodion pull request #1656: [TRAFODION-3159] Add *Syntax Descriptions and ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1656#discussion_r204862675
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -9066,13 +9066,220 @@ second argument describes the format of the 
character value.
 TO_CHAR(character-expression [,format-string])
 ```
 
+[[to_char_syntax]]
+=== Syntax Description of TO_CHAR
+
 * `_character-expression_`
 +
 is an expression that gives a datetime value.
 
 * `_format-string_`
 +
-is one of the following character string literals:
+.Details of `_format-string_`
+[cols="3*.^"]
+|===
+^| *Supported* `_character-expression_` 
+^| *`_format-string_`* 
+^| *Description* 
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'CC'
+| Two-digital century.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'D'
+| Day of week (1-7), where 1 is Sunday, 6 is Saturday. +
+This is not configurable.
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DD'
+| Day of month (01-31).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DDD'
+| Day of year (001-366).
+
+a| * `_datetime-expression_` 
+** DATE 
+** TIMESTAMP 
+| 'DY'
+a| Name of day, which is a three-letter abbreviation for the day in 
uppercase. +
+
+Following values are valid: +
--- End diff --

Suggest "The following... "


---

[GitHub] trafodion pull request #1656: [TRAFODION-3159] Add *Syntax Descriptions and ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1656#discussion_r204860415
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_functions_and_expressions.adoc ---
@@ -9098,30 +9305,606 @@ is one of the following character string literals:
 ** 'MM/DD/ HH24:MI:SS'
 ** 'DD-MON- HH:MI:SS'
 ** 'MONTH DD, , HH:MI'
-** 'DD.MM. HH24.MI.SS'
-
-Here,  refers to a 4-digit year. YY refers to a two-digit year. MM 
refers to a two-digit month. MON refers to
-a three-letter abbreviation for month ('JAN', 'FEB', 'MAR', 'APR', 'MAY', 
'JUN', 'JUL', 'AUG', 'SEP',
-'OCT', 'NOV' or 'DEC'). MONTH refers to the month spelled out. In the 
output value, the month abbreviation or month
-will appear in upper case.
+** 'DD.MM. HH24.MI.SS' 
 
-HH and HH24 refer to a 2-digit hour field. MI refers to a two-digit 
minutes field. SS refers to a 2-digit seconds field.
++
+Here:
 
-If the _format-string_ argument is omitted, '-MM-DD' is used as the
+***  refers to a four-digit year. 
+*** YY refers to a two-digit year. 
+*** MM refers to a two-digit month. 
+*** MON refers to a three-letter abbreviation for month ('JAN', 'FEB', 
'MAR', 'APR', 'MAY', 'JUN', 'JUL', 'AUG', 'SEP',
+'OCT', 'NOV' or 'DEC'). 
+*** MONTH refers to the month spelled out. In the output value, the month 
abbreviation or month
+will appear in upper case.
+*** HH and HH24 refer to a two-digit hour field. 
+*** MI refers to a two-digit minutes field. 
+*** SS refers to a two-digit seconds field.
+*** If the `_format-string_` argument is omitted, '-MM-DD' is used as 
the
 default for date and timestamp values, and 'HH:MI:SS' is used for time 
values.
 
 [[considerations_for_to_char]]
 === Considerations for TO_CHAR
 
-If the _format-string_ contains only hour, minute or seconds fields, the 
input data type must be time or timestamp.
+If the `_format-string_` contains only hour, minute or seconds fields, the 
input data type must be time or timestamp.
 
-If the _format-string_ contains only year, month or day fields, the input 
data type must be date or timestamp.
+If the `_format-string_` contains only year, month or day fields, the 
input data type must be date or timestamp.
 
-If the _format-string_ contains all fields, and the input data type is 
date, the hour, minute and second fields in the result will be filled with 
zeroes.
+If the `_format-string_` contains all fields, and the input data type is 
date, the hour, minute and second fields in the result will be filled with 
zeroes.
 
 [[examples_of_to_char]]
 === Examples of TO_CHAR
 
+[[examples_of_to_char_cc]]
+ Examples of TO_CHAR (CC)
+
+* This example converts the `_DATE_` value to the character value of 
century.
++
+```
+SQL>SELECT TO_CHAR (DATE '0001-11-02','CC') FROM DUAL;
+
+(EXPR)
+--
+01
+
+--- 1 row(s) selected.
+```
+
+* This example converts the `_TIMESTAMP_` value to the character value of 
century.
++
+```
+SQL>SELECT TO_CHAR (TIMESTAMP '9899-12-31 23:59:59','CC') FROM DUAL;
+
+(EXPR)
+--
+99
+
+--- 1 row(s) selected.
+```
+
+[[examples_of_to_char_d]]
+ Examples of TO_CHAR (D)
+
+* This example converts the `_DATE_` value to the character value of day 
of week.
++
+```
+SQL>SELECT TO_CHAR (DATE '2018-01-01','D') FROM DUAL;
+
+(EXPR)
+--
+2 
+
+--- 1 row(s) selected.
+```
+
+* This example converts the `_TIMESTAMP_` value to the character value of 
day of week.
++
+```
+SQL>SELECT TO_CHAR (TIMESTAMP '2018-04-01 23:59:59','D') FROM DUAL;
+
+(EXPR)
+--
+1 
+
+--- 1 row(s) selected.
+```
+
+[[examples_of_to_char_dd]]
+ Examples of TO_CHAR (DD)
+
+* This example converts the `_DATE_` value to the character value of day 
of month.
++
+```
+SQL>SELECT TO_CHAR (DATE '2018-01-01','DD') FROM DUAL;
+
+(EXPR)
+--
+01
+
+--- 1 row(s) selected. 
+```
+
+* This example converts the `_TIMESTAMP_` value to the character value of 
day of month.
++
+```
+SQL>SELECT TO_CHAR (TIMESTAMP '2018-12-31 23:59:59','DD') FROM DUAL;
+
+(EXPR)
 

[GitHub] trafodion pull request #1658: [TRAFODION-3155] implemented the OVERLAPS pred...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1658#discussion_r204855585
  
--- Diff: core/sql/parser/sqlparser.y ---
@@ -19430,6 +19432,124 @@ exists_predicate : TOK_EXISTS rel_subquery
  $$ = new (PARSERHEAP()) Exists($2);
}
 
+overlaps_predicate : value_expression_list_paren TOK_OVERLAPS 
value_expression_list_paren
+ {
+ItemExprList  exprList1($1, PARSERHEAP());
+ItemExprList  exprList2($3, PARSERHEAP());
+//Syntax Rules: 
+//  1) The degrees of  and  shall both be 2.
+if ((exprList1.entries() != 2)
+  || (exprList1.entries() != exprList2.entries()))
+  {
+ *SqlParser_Diags << DgSqlCode(-4077)
--- End diff --

This is OK to have in the Parser, as it depends only on syntax.


---

[GitHub] trafodion pull request #1658: [TRAFODION-3155] implemented the OVERLAPS pred...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1658#discussion_r204855399
  
--- Diff: core/sql/parser/sqlparser.y ---
@@ -19430,6 +19432,124 @@ exists_predicate : TOK_EXISTS rel_subquery
  $$ = new (PARSERHEAP()) Exists($2);
}
 
+overlaps_predicate : value_expression_list_paren TOK_OVERLAPS 
value_expression_list_paren
+ {
+ItemExprList  exprList1($1, PARSERHEAP());
+ItemExprList  exprList2($3, PARSERHEAP());
+//Syntax Rules: 
+//  1) The degrees of  and  shall both be 2.
+if ((exprList1.entries() != 2)
+  || (exprList1.entries() != exprList2.entries()))
+  {
+ *SqlParser_Diags << DgSqlCode(-4077)
+ << DgString0("OVERLAPS");
+ YYERROR;  // CHANGE TO YYABORT
+  }
+
+ItemExpr *D1 = (*$1)[0];
+ItemExpr *E1 = (*$1)[1];
+ItemExpr *D2 = (*$3)[0];
+ItemExpr *E2 = (*$3)[1];
+
+NABoolean negate = FALSE;
+ItemExpr *tmpItemExpr = E1;
--- End diff --

This logic should be in the Binder, not in the Parser. The Parser does not 
know the datatypes of anything except constants. And I don't think you want to 
limit OVERLAPS to operating only on constants.


---

[GitHub] trafodion pull request #1660: [TRAFODION-3158] Make EncodedValue error handl...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1660

[TRAFODION-3158] Make EncodedValue error handling more robust

This pull request makes two code changes to function 
EncodedValue::constructorFunction.

1. Formerly, the function would check for the existence of an error in 
CmpCommon::diags() and remove it if present. This wasn't sufficient though as 
the test case in the JIRA generated *two* errors in CmpCommon::diags(); the 
code as it was removed only one of them. The result was that the Normalizer 
retried the query, and the warnings concerning the bad histogram values were 
lost.

2. In my initial attempt to fix this problem, I put an unguarded call to 
CmpCommon::diags()->mark() to capture the state of CmpCommon::diags() on entry 
to EncodedValue::constructorFunction. This turned out to cause cores in the 
mxssmp process at sqstart time, because in that process it happened that the 
EncodedValue constructor for a global object was called before the global 
CmpCommon::diags() was initialized. This could have happened in any process; 
it's just the luck of the draw in what order the linker decides to invoke 
global constructors. So to fix that problem, I was forced to add a flag so that 
code paths involving global objects do not try to dereference 
CmpCommon::diags(). This change is to my mind ugly; I added extensive comments 
explaining the situation. Some significant refactoring will be required to 
produce a more elegant solution.



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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3158

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

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


commit 64e5ed11a2d1deae8d748d7d749abb5007ae0c41
Author: Dave Birdsall 
Date:   2018-07-23T17:24:57Z

[TRAFODION-3158] Make EncodedValue error handling more robust




---

[GitHub] trafodion pull request #1645: [TRAFODION-3144] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1645#discussion_r203555447
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -390,25 +390,27 @@ alter-action is:
 
 column-definition is:
column-name data-type
-  ([DEFAULT default]
- [[constraint constraint-name] column-constraint])
+  [[DEFAULT default]
+ [[constraint constraint-name] column-constraint]]
 
 data-type is:
- char[acter] [(length)[characters]]
- [CHARACTER SET char-set-name]
- [UPSHIFT] [[not] casespecific]
-   | char[acter] varying (length)
- [character set char-set-name]
- [upshift] [[not] casespecific]
-   | varchar (length) [character set char-set-name]
- [upshift] [[not] casespecific]
-   | varchar2 (length) [character set char-set-name]
- [upshift] [[not] casespecific] 
+ char[acter] [(length) [characters]]
+ [CHARACTER SET char-set-name]
+ [UPSHIFT] [[NOT] CASESPECIFIC]
+   | char[acter] varying (length [characters])
+ [CHARACTER SET char-set-name]
+ [UPSHIFT] [[NOT] CASESPECIFIC]
+   | varchar (length [characters]) 
+ [CHARACTER SET char-set-name]
+ [UPSHIFT] [[NOT] CASESPECIFIC]
+   | varchar2 (length [characters]) 
+  [CHARACTER SET char-set-name]
+  [UPSHIFT] [[NOT] CASESPECIFIC] 
+   | nchar [(length) [characters]]
--- End diff --

Same here.


---

[GitHub] trafodion pull request #1645: [TRAFODION-3144] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1645#discussion_r203555402
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -390,25 +390,27 @@ alter-action is:
 
 column-definition is:
column-name data-type
-  ([DEFAULT default]
- [[constraint constraint-name] column-constraint])
+  [[DEFAULT default]
+ [[constraint constraint-name] column-constraint]]
 
 data-type is:
- char[acter] [(length)[characters]]
- [CHARACTER SET char-set-name]
- [UPSHIFT] [[not] casespecific]
-   | char[acter] varying (length)
- [character set char-set-name]
- [upshift] [[not] casespecific]
-   | varchar (length) [character set char-set-name]
- [upshift] [[not] casespecific]
-   | varchar2 (length) [character set char-set-name]
- [upshift] [[not] casespecific] 
+ char[acter] [(length) [characters]]
--- End diff --

Should be "char[acter] [(length [characters])]". The optional [characters] 
syntactic element should be inside the parenthesis.


---

[GitHub] trafodion pull request #1644: [TRAFODION-3142] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1644#discussion_r203554845
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -2963,18 +2963,22 @@ column-definition is:
 <<<
 ```
 data-type is:
-char[acter] [(length [characters])]
-  [character set char-set-name]
-  [upshift] [[not]casespecific]
+char[acter] [(length) [characters]]
--- End diff --

Should be "char[acter] [(length [characters])]"  (The trailing parenthesis 
should come after the optional [characters] syntactic element)


---

[GitHub] trafodion pull request #1644: [TRAFODION-3142] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1644#discussion_r203554900
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -2963,18 +2963,22 @@ column-definition is:
 <<<
 ```
 data-type is:
-char[acter] [(length [characters])]
-  [character set char-set-name]
-  [upshift] [[not]casespecific]
+char[acter] [(length) [characters]]
+[character set char-set-name]
+[upshift] [[not]casespecific]
   | char[acter] varying (length [characters])
-  [character set char-set-name]
-  [upshift] [[not]casespecific]
-  | varchar (length) [character set char-set-name]
+[character set char-set-name]
+[upshift] [[not]casespecific]
+  | varchar (length [characters]) 
+[character set char-set-name]
+[upshift] [[not]casespecific]
+  | varchar2 (length [characters]) 
+ [character set char-set-name]
+ [upshift] [[not]casespecific]  
+  | nchar [(length) [characters]]
--- End diff --

Same comment here.


---

[GitHub] trafodion pull request #1652: [TRAFODION-3153] Fix issue with CREATE EXTERNA...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1652

[TRAFODION-3153] Fix issue with CREATE EXTERNAL TABLE

Change the parser production for CREATE EXTERNAL TABLE to point to the 
proper token for the optional IF NOT EXISTS clause.

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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3153

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

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


commit 133fc53973cbd82f12713217f1e00c6a71190dde
Author: Dave Birdsall 
Date:   2018-07-18T22:05:38Z

[TRAFODION-3153] Fix issue with CREATE EXTERNAL TABLE




---

[GitHub] trafodion pull request #1645: [TRAFODION-3144] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1645#discussion_r203094486
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -390,25 +390,27 @@ alter-action is:
 
 column-definition is:
column-name data-type
-  ([DEFAULT default]
- [[constraint constraint-name] column-constraint])
+  [[DEFAULT default]
+ [[constraint constraint-name] column-constraint]]
 
 data-type is:
  char[acter] [(length)[characters]]
- [CHARACTER SET char-set-name]
- [UPSHIFT] [[not] casespecific]
-   | char[acter] varying (length)
- [character set char-set-name]
- [upshift] [[not] casespecific]
-   | varchar (length) [character set char-set-name]
- [upshift] [[not] casespecific]
-   | varchar2 (length) [character set char-set-name]
- [upshift] [[not] casespecific] 
+ [CHARACTER SET char-set-name]
+ [UPSHIFT] [[NOT] CASESPECIFIC]
+   | char[acter] varying (length [characters])
+ [CHARACTER SET char-set-name]
+ [UPSHIFT] [[NOT] CASESPECIFIC]
+   | varchar (length [characters]) 
+ [CHARACTER SET char-set-name]
+ [UPSHIFT] [[NOT] CASESPECIFIC]
+   | varchar2 (length [characters]) 
+  [CHARACTER SET char-set-name]
+  [UPSHIFT] [[NOT] CASESPECIFIC] 
+   | nchar [length [characters]]
--- End diff --

Thanks; you are correct. So, it should say, "nchar [(length 
[characters])]". The length is optional, but if it is present, parentheses are 
required.


---

[GitHub] trafodion pull request #1644: [TRAFODION-3142] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1644#discussion_r203093950
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -2963,18 +2963,22 @@ column-definition is:
 <<<
 ```
 data-type is:
-char[acter] [(length [characters])]
-  [character set char-set-name]
-  [upshift] [[not]casespecific]
+char[acter] [length [characters]]
--- End diff --

FYI: I double-checked the parser code. Length is optional for CHAR and 
NCHAR. Length is required for VARCHAR and NCHAR VARYING.


---

[GitHub] trafodion pull request #1644: [TRAFODION-3142] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1644#discussion_r203091243
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -2963,18 +2963,22 @@ column-definition is:
 <<<
 ```
 data-type is:
-char[acter] [(length [characters])]
-  [character set char-set-name]
-  [upshift] [[not]casespecific]
+char[acter] [length [characters]]
--- End diff --

Ah, good catch! I stand corrected. So, the original text, "char[acter] 
[(length [characters])] was correct? (The parentheses are needed if a length is 
supplied.)


---

[GitHub] trafodion pull request #1642: [TRAFODION-3136] New featrue: improve on split...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1642#discussion_r202838056
  
--- Diff: core/sql/bin/SqlciErrors.txt ---
@@ -1450,6 +1450,7 @@ $1~String1 
 4494 Z 9 BEGINNER MINOR LOGONLY LOAD is not supported on a table 
with LOB columns. Table $0~TableName has LOB column $1~ColumnName.
 4495 Z 9 BEGINNER MINOR LOGONLY UNLOAD is not supported on a 
SELECT with LOB columns. $0~ColumnName is a LOB column.
 4496 42000 9 BEGINNER MAJOR DBADMIN $0~String0 is supported only with 
datetime fields.
+4497 Z 9 BEGINNER MAJOR DBADMIN The $0~String0 operand of 
$1~String1 must be greater than $2~String2
--- End diff --

Minor point. Please add a period at the end of your new message text. Also, 
as I mentioned below, since this is a run-time message, it should be in the 
8000-8999 range.


---

[GitHub] trafodion pull request #1642: [TRAFODION-3136] New featrue: improve on split...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1642#discussion_r202841083
  
--- Diff: core/sql/exp/exp_function.cpp ---
@@ -2746,6 +2754,59 @@ ex_expr::exp_return_type 
ex_function_unixtime::eval(char *op_data[],
   return ex_expr::EXPR_OK;
 }
 
+ex_expr::exp_return_type ex_function_split_part::eval(char *op_data[]
+   , CollHeap* heap
+   , ComDiagsArea** diagsArea)
+{
+  size_t sourceLen = getOperand(1)->getLength(op_data[-MAX_OPERANDS+1]);
+  size_t patternLen = getOperand(2)->getLength(op_data[-MAX_OPERANDS+2]);
+  Lng32 indexOfTarget = *(Lng32 *)op_data[3];
+
+  if (indexOfTarget <= 0)
+{
+   ExRaiseSqlError(heap, diagsArea, (ExeErrorCode)4497);
--- End diff --

Since this is a run-time error, it should be in the 8000-8999 range. Also, 
there is an enum file for these error numbers. See 
https://cwiki.apache.org/confluence/display/TRAFODION/How+to+Add+A+New+SQL+Error+Diagnostic.


---

[GitHub] trafodion pull request #1642: [TRAFODION-3136] New featrue: improve on split...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1642#discussion_r202841611
  
--- Diff: docs/messages_guide/src/asciidoc/_chapters/binder_msgs.adoc ---
@@ -2919,3 +2919,15 @@ Use of predefined UDF  is deprecated and this 
function will be removed in
 
 *Recovery:* See message.
 
+[[SQL-4997]]
--- End diff --

Looks like you meant 4497 rather than 4997. But as mentioned above, since 
this is a run-time error, the error message number should be in the range 
8000-8999.


---

[GitHub] trafodion pull request #1642: [TRAFODION-3136] New featrue: improve on split...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1642#discussion_r202842026
  
--- Diff: docs/messages_guide/src/asciidoc/_chapters/binder_msgs.adoc ---
@@ -2919,3 +2919,15 @@ Use of predefined UDF  is deprecated and this 
function will be removed in
 
 *Recovery:* See message.
 
+[[SQL-4997]]
+== SQL 4997
+
+```
+The  operand of builtin-func  must be great than .
+```
+*Cause:* You attempted to use the random function, which some operand of 
this has range.
--- End diff --

The mention of the "random" function looks like a cut-and-paste error. 
Suggested wordsmith: "You attempted to use an invalid value for the given 
operand of the given function."


---

[GitHub] trafodion pull request #1642: [TRAFODION-3136] New featrue: improve on split...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1642#discussion_r202839688
  
--- Diff: core/sql/optimizer/SynthType.cpp ---
@@ -7139,3 +7139,79 @@ const NAType * ItmLeadOlapFunction::synthesizeType()
return result;
 }
 
+const NAType * SplitPart::synthesizeType()
+{
+  ValueId vid1 = child(0)->getValueId(); 
+  ValueId vid2 = child(1)->getValueId();
+  ValueId vid3 = child(2)->getValueId();
+  vid1.coerceType(NA_CHARACTER_TYPE);
+  vid2.coerceType(NA_CHARACTER_TYPE);
+  SQLInt si(NULL);
+  vid3.coerceType(NA_NUMERIC_TYPE);
+
+  const NAType *operand1 = &child(0)->getValueId().getType();
+  const NAType *operand2 = &child(1)->getValueId().getType();
+  const NAType *operand3 = &child(2)->getValueId().getType();
+
+  if ((operand1->getTypeQualifier() != NA_CHARACTER_TYPE) 
+  && (operand1->getFSDatatype() != REC_CLOB))
+  {
+//4051 The first operand of a split_part function must be character.
+*CmpCommon::diags()<getTypeQualifier() != NA_CHARACTER_TYPE)
+  && (operand1->getFSDatatype() != REC_CLOB))
+  {
+//4051 The second operand of a split_part function must be character.
+*CmpCommon::diags()<

[GitHub] trafodion pull request #1649: [TRAFODION-3148] Fix UPDATE STATS bug on Hive ...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1649

[TRAFODION-3148] Fix UPDATE STATS bug on Hive tables w/ column _DIVISION_1_

This fixes a bug concerning sample tables created by UPDATE STATISTICS on 
Hive tables.

When a Hive table had a column named "_DIVISION_1_" (or any other reserved 
Trafodion column name), the UPDATE STATISTICS command would fail if a sample 
table needed to be created. The failure would occur because we would not 
include "_DIVISION_1_" etc. in the select list of the UPSERT/SELECT statement 
used to populate the sample table.

This has been fixed.

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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3148

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

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


commit d3945fe7973cd001b3b5268e53b4d1117f45d54f
Author: Dave Birdsall 
Date:   2018-07-16T20:41:21Z

[TRAFODION-3148] Fix UPDATE STATS bug on Hive tables w/ column _DIVISION_1_




---

[GitHub] trafodion pull request #1645: [TRAFODION-3144] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1645#discussion_r202732987
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -390,25 +390,27 @@ alter-action is:
 
 column-definition is:
column-name data-type
-  ([DEFAULT default]
- [[constraint constraint-name] column-constraint])
+  [[DEFAULT default]
+ [[constraint constraint-name] column-constraint]]
 
 data-type is:
  char[acter] [(length)[characters]]
- [CHARACTER SET char-set-name]
- [UPSHIFT] [[not] casespecific]
-   | char[acter] varying (length)
- [character set char-set-name]
- [upshift] [[not] casespecific]
-   | varchar (length) [character set char-set-name]
- [upshift] [[not] casespecific]
-   | varchar2 (length) [character set char-set-name]
- [upshift] [[not] casespecific] 
+ [CHARACTER SET char-set-name]
+ [UPSHIFT] [[NOT] CASESPECIFIC]
+   | char[acter] varying (length [characters])
+ [CHARACTER SET char-set-name]
+ [UPSHIFT] [[NOT] CASESPECIFIC]
+   | varchar (length [characters]) 
+ [CHARACTER SET char-set-name]
+ [UPSHIFT] [[NOT] CASESPECIFIC]
+   | varchar2 (length [characters]) 
+  [CHARACTER SET char-set-name]
+  [UPSHIFT] [[NOT] CASESPECIFIC] 
+   | nchar [length [characters]]
--- End diff --

This should say, "nchar (length [characters])"


---

[GitHub] trafodion pull request #1645: [TRAFODION-3144] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1645#discussion_r202733241
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -756,6 +758,86 @@ Alter TABLE PRODUCT
 ALTER COLUMN vend_id RENAME TO cstm_id;
 ```
 
+* This example demonstrates the character set of NCHAR column cannot be 
altered.
--- End diff --

See my comments on https://github.com/apache/trafodion/pull/1644. 


---

[GitHub] trafodion pull request #1645: [TRAFODION-3144] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1645#discussion_r202732750
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -390,25 +390,27 @@ alter-action is:
 
 column-definition is:
column-name data-type
-  ([DEFAULT default]
- [[constraint constraint-name] column-constraint])
+  [[DEFAULT default]
+ [[constraint constraint-name] column-constraint]]
 
 data-type is:
  char[acter] [(length)[characters]]
--- End diff --

This should say, "char[acter] (length [characters])"


---

[GitHub] trafodion pull request #1644: [TRAFODION-3142] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1644#discussion_r202728022
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -2963,18 +2963,22 @@ column-definition is:
 <<<
 ```
 data-type is:
-char[acter] [(length [characters])]
-  [character set char-set-name]
-  [upshift] [[not]casespecific]
+char[acter] [length [characters]]
+[character set char-set-name]
+[upshift] [[not]casespecific]
   | char[acter] varying (length [characters])
-  [character set char-set-name]
-  [upshift] [[not]casespecific]
-  | varchar (length) [character set char-set-name]
+[character set char-set-name]
+[upshift] [[not]casespecific]
+  | varchar (length [characters]) 
+[character set char-set-name]
+[upshift] [[not]casespecific]
+  | varchar2 (length [characters]) 
+ [character set char-set-name]
+ [upshift] [[not]casespecific]  
+  | nchar [length [characters]]
--- End diff --

Should be "nchar (length [characters])"


---

[GitHub] trafodion pull request #1644: [TRAFODION-3142] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1644#discussion_r202731971
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -3712,6 +3716,60 @@ ATTRIBUTES ALIGNED FORMAT
 --- SQL operation complete.
 ```
 
+* This example demonstrates the character set of NCHAR column (_c1_) 
*cannot be specified*.
--- End diff --

I think one example is probably enough. The point we are trying to make is 
that "NATIONAL CHAR" implies a particular character set, namely, "UCS2". This 
is hard-coded into the product; it cannot be changed. Possible word-smith: 
"NATIONAL CHAR (or NCHAR) is a short-hand for CHAR ... CHARACTER SET UCS2. So 
it is not possible to specify a CHARACTER SET clause when using NCHAR, as the 
following example shows:"


---

[GitHub] trafodion pull request #1644: [TRAFODION-3142] Correct Syntactic Description...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1644#discussion_r202727823
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -2963,18 +2963,22 @@ column-definition is:
 <<<
 ```
 data-type is:
-char[acter] [(length [characters])]
-  [character set char-set-name]
-  [upshift] [[not]casespecific]
+char[acter] [length [characters]]
--- End diff --

I think you meant, "char[acter] (length [characters])"  (That is, the 
length is surrounded by parentheses, and it is not optional.)


---

[GitHub] trafodion pull request #1641: [TRAFODION-3140] Update QQ ID and other news o...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1641

[TRAFODION-3140] Update QQ ID and other news on Apache Trafodion main page

This change updates the QQ ID for Chinese discussions on the Apache 
Trafodion main page.

It also updates the news section of the page.

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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3140

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

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


commit 6de171c6af3c09b6a2d66a29fcf4ffd5e0e6afa3
Author: Dave Birdsall 
Date:   2018-07-12T21:01:31Z

[TRAFODION-3140] Update QQ ID and other news on Apache Trafodion main page




---

[GitHub] trafodion pull request #1640: [TRAFODION-3135] Remove anachronisms from keyw...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1640

[TRAFODION-3135] Remove anachronisms from keyword tables

This set of changes removes references to reserved words from a predecessor 
product from key word tables in parser/ParKeyWords.cpp and 
common/ComResWords.cpp. A few other such references in other code was removed 
also.

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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3135

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

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


commit 9fc2ebbdd6613d0c3bb86dc30a55608b4830c708
Author: Dave Birdsall 
Date:   2018-07-12T00:45:47Z

[TRAFODION-3135] Remove anachronisms from keyword tables




---

[GitHub] trafodion pull request #1639: [TRAFODION-3138] Part 1 of removing static SQL...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1639

[TRAFODION-3138] Part 1 of removing static SQL code from ustats + one bug 
fix

This pull request contains two changes:

1. It removes obsolete static SQL code from two ustat modules (hs_read.cpp 
and hs_update.cpp). It also removes obsolete metadata version checking code. 
Thanks to @anoopsharma00 for these changes.

2. It fixes a bug in incremental UPDATE STATISTICS. If one has a table name 
containing a dot (.), incremental UPDATE STATISTICS fails. For example:

```
>>update statistics for table "A.B" on existing columns incremental where a 
> 128;

*** ERROR[15005] Unmatched quote in input (unterminated string): 
select count(*) from B" where  a > 128;

--- SQL operation failed with errors.
>>
```

This bug has been fixed (hs_globals.cpp). Thanks to @anoopsharma00 also for 
this change.

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

    $ git pull https://github.com/DaveBirdsall/trafodion Trafodion3138

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

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


commit 2358b6f6b5e894ccc626fa505eea04c572dacabd
Author: Dave Birdsall 
Date:   2018-07-11T20:39:34Z

[TRAFODION-3138] Part 1 of removing static SQL code from ustats + one bug 
fix




---

[GitHub] trafodion pull request #1633: [TRAFODION-2743] Windows ODBC LOB support

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1633#discussion_r201728736
  
--- Diff: win-odbc64/odbcclient/drvr35/lob.cpp ---
@@ -54,8 +54,12 @@ namespace ODBC {
 CStmt *pStatement = (CStmt *)hstmt;
 IDL_long extractLen = length;
 
+IDL_string lobHandleTemp = (IDL_string)malloc(lobHandleLen + 1);
--- End diff --

Is there a place where this string will be deleted? If not, is this a 
memory leak?


---

[GitHub] trafodion pull request #1637: [TRAFODION-3131] Fix some reserved and non-res...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1637

[TRAFODION-3131] Fix some reserved and non-reserved word issues

This set of changes does the following:

1. Fixes some issues with reserved words not being recognized as reserved 
words.
2. Updates core/TEST037 to include test cases for some recently added 
functions (CRC32, MD5, SHA, SHA1 and SHA2)
3. Updates the expected results for core/TEST037.

Note: I'm aware that the comments in core/TEST037 are not perfectly aligned 
with the expected results. (But they are much more aligned than they were.) 
Will leave further improvements to follow-up work.

Note: I considered refactoring common/ComResWords.cpp and the "prettify SQL 
text" functions in common/NAString.cpp, moving them to the parser and changing 
them to use the ParKeyWords table in parser/ParKeyWords.cpp. I wanted to do 
this to simplify development, consolidating three keyword tables into one. I 
started down this path and discovered that this was far from easy; moreover I 
concluded that it wouldn't actually simplify development much. So I fell back 
to the position of merely adding thorough documentation to 
parser/ParKeyWords.cpp, giving instructions on how to keep these tables in sync.

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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3131

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

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


commit 8ed5ca2070d28e6a331ded03ea1a02af5e15c932
Author: Dave Birdsall 
Date:   2018-07-10T21:51:28Z

[TRAFODION-3131] Fix some reserved and non-reserved word issues




---

[GitHub] trafodion pull request #1632: [TRAFODION-3127] tochar enhancement

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1632#discussion_r201484661
  
--- Diff: core/sql/exp/exp_datetime.cpp ---
@@ -3294,6 +3338,39 @@ convertMonthToStr(Lng32 value, char *&result, UInt32 
width)
   result += width;
 }
 
+static void
+convertDayOfWeekToStr(Lng32 value, char *&result, NABoolean bAbbreviation, 
UInt32 width)
+{
+  const char* dayofweek[] =
+  {
+"SUNDAY   ",
+"MONDAY   ",
+"TUESDAY  ",
+"WEDNESDAY",
+"THURSDAY ",
+"FRIDAY   ",
+"SATURDAY "
+  };
+
+  const char* dayofweek_abb[] =
+  {
+"SUN",
+"MON",
+"TUE",
+"WED",
+"THU",
+"FRI",
+"SAT"
+  };
+
+  if (bAbbreviation)
+strcpy(result, dayofweek_abb[value-1]);
+  else
+strcpy(result, dayofweek[value-1]);
+  // Update result pointer to point to end of string.
+  result += width;
--- End diff --

Why width instead of strlen(result)? (This is probably OK; I'm just 
curious.)


---

[GitHub] trafodion pull request #1633: [TRAFODION-2743] Windows ODBC LOB support

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1633#discussion_r201439488
  
--- Diff: win-odbc64/Common/TransportBase.h ---
@@ -134,6 +134,8 @@ enum SRVR_API {
SRVR_API_SQLFASTEXECDIRECT, //OK WMS
SRVR_API_SQLFASTFETCH_PERF, //OK WMS
SRVR_API_GETSEGMENTS,   //OK WMS
+SRVR_API_EXTRACTLOB,//OK LOB
+SRVR_API_UPDATELOB, //OK LOB
--- End diff --

My guess is that the original code uses tabs (which is bad) and the tab 
setting in github assumes 8 spaces per tab. Consider changing your editor to 
use 8 spaces per tab.


---

[GitHub] trafodion pull request #1633: [TRAFODION-2743] Windows ODBC LOB support

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1633#discussion_r201149361
  
--- Diff: win-odbc64/Common/TransportBase.h ---
@@ -134,6 +134,8 @@ enum SRVR_API {
SRVR_API_SQLFASTEXECDIRECT, //OK WMS
SRVR_API_SQLFASTFETCH_PERF, //OK WMS
SRVR_API_GETSEGMENTS,   //OK WMS
+SRVR_API_EXTRACTLOB,//OK LOB
+SRVR_API_UPDATELOB, //OK LOB
--- End diff --

You may want to check the tab settings in your editor; the indentation 
seems to frequently be wrong. Best practice is to store spaces instead of tabs.


---

[GitHub] trafodion pull request #1634: [TRAFODION-3132] Add and Correct Descriptions ...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1634#discussion_r201148299
  
--- Diff: 
docs/sql_reference/src/asciidoc/_chapters/sql_language_elements.adoc ---
@@ -396,21 +396,22 @@ length of 8 bytes.
 * _precision_ specifies the allowed number of decimal digits.
 
 
-1.  The size of a column that allows null values is 2 bytes larger than 
the size for the defined data type.
-2.  The maximum row size is 32708 bytes, but the actual row size is less 
than that because of bytes used by
-null indicators, varchar column length indicators, and actual data 
encoding.
-3.  Storage size is the same as that required by CHAR data type but store 
only half as many characters depending
-on character set selection.
-4.  Storage size is reduced by 4 bytes for storage of the varying 
character length.
-5.  The maximum number of digits in an INTERVAL value is 18, including the 
digits in all INTERVAL fields of the value.
+1. The size of a column that allows null values is 2 bytes larger than the 
size for the defined data type.
+2. Storage size is the same as that required by CHAR data type but store 
only half.
--- End diff --

Perhaps what you mean in footnote 2 is: "The maximum number of characters 
depends on the character set. For 8-bit character sets such as ISO88591, the 
maximum number of characters is equal to the maximum storage size in bytes. For 
16-bit character sets such as UCS2, the maximum number of characters is half 
the maximum storage size in bytes. For character sets requiring up to 32 bits 
per character such as UTF8, the maximum number of characters is one fourth the 
maximum storage size in bytes."


---

[GitHub] trafodion pull request #1635: [TRAFODION-2926] Fix issue-tracking URL on web...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1635

[TRAFODION-2926] Fix issue-tracking URL on web site

Change the URL from http: to https:

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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion2926

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

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


commit 044306bbc8f2723a6eea08f23289aebc890c3c93
Author: Dave Birdsall 
Date:   2018-07-09T18:00:38Z

[TRAFODION-2926] Fix issue-tracking URL on web site




---

[GitHub] trafodion pull request #1630: [TRAFODION 3128] Fix issue with interval liter...

[DaveBirdsall]

GitHub user DaveBirdsall opened a pull request:

https://github.com/apache/trafodion/pull/1630

[TRAFODION 3128] Fix issue with interval literals in multi-column histograms

When an INTERVAL SECOND(m,n) column participates in a multi-column 
histogram, EncodedValue::constructorFunction (optimizer/EncodedValue.cpp) did 
not properly scan for the comma delimiter between column values, resulting in 
6003 warnings. This has been fixed; we now ignore commas within sets of 
parentheses.

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

$ git pull https://github.com/DaveBirdsall/trafodion Trafodion3128

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

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


commit 598cd634b09e598580a5d271db9e3f040ac8fdcb
Author: Dave Birdsall 
Date:   2018-07-05T16:16:38Z

[TRAFODION 3128] Fix issue with interval literals in multi-column histograms




---

[GitHub] trafodion pull request #1587: [TRAFODION-3094] Add Syntax and Examples for *...

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1587#discussion_r199651804
  
--- Diff: docs/sql_reference/src/asciidoc/_chapters/sql_statements.adoc ---
@@ -4342,6 +4363,45 @@ DELETE FROM invent.partsupp WHERE suppnum NOT IN
 delete from table1 where a in (select a from table1 where b > 200)
 ```
 
+* This example executes a non-transactional delete of first 1000 rows that 
satisfy the
+WHERE condition.
++
+```
+DELETE WITH NO ROLLBACK [FIRST 1000] 
+FROM test 
+WHERE id<30;
+```
+
+* This example shows that the option `WITH NO ROLLBACK` is only supported 
if there is no
+index on the table.
++
+```
+SQL>DELETE WITH NO ROLLBACK FROM test;
+
+*** ERROR[3232] INSERT, UPDATE, or DELETE of table TRAFODION.SEABASE.TEST 
with dependent Index : TRAFODION.SEABASE.INDEX_TEST is not allowed when NO 
ROLLBACK transaction setting is in effect. [2018-05-30 05:42:47]
--- End diff --

Keeping it is fine.


---

[GitHub] trafodion pull request #1622: [TRAFODION-3118] Improve on parts of EXTRACT

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1622#discussion_r199649187
  
--- Diff: core/sql/parser/sqlparser.y ---
@@ -500,6 +501,7 @@ static void enableMakeQuotedStringISO88591Mechanism()
 %token  TOK_CARDINALITY
 %token  TOK_CASE
 %token  TOK_CAST
+%token  TOK_CENTURY
--- End diff --

The new tokens, TOK_CENTURY, TOK_DECADE, etc., need to be added to the 
nonreserved_word production, otherwise they will be taken as reserved words 
(notwithstanding the NONRESTOKEN_ flag in ParKeyWords.cpp). Please also add 
them to test core/TEST037, which tests that non-reserved words really are 
non-reserved.


---

[GitHub] trafodion pull request #1622: [TRAFODION-3118] Improve on parts of EXTRACT

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1622#discussion_r199650319
  
--- Diff: core/sql/optimizer/SynthType.cpp ---
@@ -4416,6 +4416,22 @@ const NAType *Extract::synthesizeType()
 
 return NULL;
   }
+  if ( type != NA_DATETIME_TYPE )
+{
+  enum rec_datetime_field eField = getExtractField();
+  NAString sErr;
+  if ( REC_DATE_WEEK == eField
+  || REC_DATE_DOW == eField
+  || REC_DATE_DOY == eField
+  || REC_DATE_WOM == eField
+  || REC_DATE_CENTURY == eField)
--- End diff --

Should DECADE be in this list too?


---

[GitHub] trafodion pull request #1622: [TRAFODION-3118] Improve on parts of EXTRACT

[DaveBirdsall]

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

https://github.com/apache/trafodion/pull/1622#discussion_r199649960
  
--- Diff: core/sql/regress/seabase/TEST030 ---
@@ -98,6 +98,11 @@ select HOUR(interval '5:2:15:36.33' day to second(2)) 
from (values(1)) as t(a);
 select MINUTE(interval '5:13:25:2.12' day to second(2)) from (values(1)) 
as t(a);
 select extract (year from INTERVAL '97-02' YEAR TO MONTH) from (values 
(1)) as t(a);
 select interval '8' year / 4 from dual;
+select extract(week from date '2005-01-01') from (values(1)) as t(a);
+select extract(epoch from timestamp '2000-12-30 20:38:40.12') from 
(values(1)) as t(a);
+select extract(dow from timestamp '2018-06-21 20:38:40') from (values(1)) 
as t(a);
+select extract(doy from timestamp '2018-06-21 20:38:40') from (values(1)) 
as t(a);
+select extract(wom from timestamp '2018-06-21 20:38:40') from (values(1)) 
as t(a);
--- End diff --

Consider adding tests for the other options, e.g. CENTURY, DECADE


---