You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by traflm <gi...@git.apache.org> on 2018/02/15 03:10:53 UTC
[GitHub] trafodion pull request #1444: [TRAFODION-2157] add MySQL function unix_times...
GitHub user traflm opened a pull request:
https://github.com/apache/trafodion/pull/1444
[TRAFODION-2157] add MySQL function unix_timestamp,uuid,sleep
This is to add three MySQL compatible functions into Trafodion.
It is required for another JIRA to add unix_timestamp, uuid as default value of a column definition.
sleep is simple to add, so add with this change as well.
The document will be updated for a separate JIRA asap.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/traflm/trafodion TRAFODION-2157
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1444.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 #1444
----
commit dbe4cac9c8e58c8dc9c6b86b5c473320df5c44c3
Author: Liu Ming <ov...@...>
Date: 2018-02-14T08:57:58Z
[TRAFODION-2157] add MySQL function unix_timestamp,uuid,sleep
commit 3555983cdeb03059277913babd646400f5f319ff
Author: Liu Ming <ov...@...>
Date: 2018-02-14T08:58:30Z
Merge branch 'master' of git://git.apache.org/trafodion into TRAFODION-2157
commit 3c6bce6e10e8f319bcdeffa272ffc4ac31a74f48
Author: Liu Ming <ov...@...>
Date: 2018-02-14T21:51:10Z
[TRAFODION-2157] fix various issues
----
---
[GitHub] trafodion pull request #1444: [TRAFODION-2954] add MySQL function unix_times...
Posted by traflm <gi...@git.apache.org>.
Github user traflm commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1444#discussion_r169868459
--- Diff: core/sql/regress/executor/TEST002 ---
@@ -1188,6 +1188,38 @@ select * from regexp_test where c1 regexp '^(25[0-5]|2[0-4][0-9]|[0-1]{1}[0-9]{2
select * from regexp_test where c1 regexp '(中文测试)';
select * from regexp_test where c1 regexp '[^\';
drop table regexp_test;
+
+--create table have 1K rows
+create table T002T1K (uniq int not null,
+ c1K int, c100 int,
+ c10 int, c1 int, c0 int )
+ STORE BY (uniq)
+ ATTRIBUTES ALIGNED FORMAT
+ SALT USING 8 PARTITIONS
+ ;
+
+upsert using load into T002T1K select
+0 + (1000 * x10) + (100 * x1) + (10 * x1) + (1 * x01),
+0 + (100 * x10) + (10 * x1) + (1 * x01),
+0 + (10 * x1) + (1 * x01),
+0 + (1 * x01),
+0,
+X01
+from (values(0)) t
+transpose 0,1,2,3,4,5,6,7,8,9 as x10
+transpose 0,1,2,3,4,5,6,7,8,9 as x1
+transpose 0,1,2,3,4,5,6,7,8,9 as X01;
+
+create table t002timert (c0 int, c1 int, c2 largeint);
+create table t002tmp1 (c1 int);
+insert into t002tmp1 values(1),(2),(3);
+
+insert into t002timert select 1, sleep(5) , unix_timestamp() from t002tmp1;
+insert into t002timert select 2, sleep(5) , unix_timestamp() from t002tmp1;
+select 'sleeptimetest002', di from (select ( max(c2) - min(c2)) as di from t002timert ) where di between 5 and 9;
--- End diff --
above two INSERTS will insert two different unix_timestamp() , the difference is the second between their run, so if the sleep(5) eval once, the diff should be around 5 seconds, and less than 10. So the query is using 'between 5 and 9'. If something wrong, this test query will return 0 rows and fail the test.
---
[GitHub] trafodion pull request #1444: [TRAFODION-2157] add MySQL function unix_times...
Posted by traflm <gi...@git.apache.org>.
Github user traflm commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1444#discussion_r168649121
--- Diff: core/sql/exp/exp_function.cpp ---
@@ -2501,6 +2518,100 @@ ex_expr::exp_return_type ExFunctionReverseStr::eval(char *op_data[],
return ex_expr::EXPR_OK;
};
+ex_expr::exp_return_type ex_function_sleep::eval(char *op_data[],
+ CollHeap* heap,
+ ComDiagsArea** diagsArea)
+{
+ Int32 sec = 0;
+ switch (getOperand(1)->getDatatype())
+ {
+ case REC_BIN8_SIGNED:
+ sec = *(Int8 *)op_data[1] ;
+ if(sec < 0 )
+ {
+ ExRaiseSqlError(heap, diagsArea, EXE_BAD_ARG_TO_MATH_FUNC);
+ *(*diagsArea) << DgString0("SLEEP");
+ return ex_expr::EXPR_ERROR;
+ }
+ sleep(sec);
+ *(Int64 *)op_data[0] = 1;
+ break;
+
+ case REC_BIN16_SIGNED:
+ sec = *(short *)op_data[1] ;
+ if(sec < 0 )
+ {
+ ExRaiseSqlError(heap, diagsArea, EXE_BAD_ARG_TO_MATH_FUNC);
+ *(*diagsArea) << DgString0("SLEEP");
+ return ex_expr::EXPR_ERROR;
+ }
+ sleep(sec);
+ *(Int64 *)op_data[0] = 1;
+ break;
+
+ case REC_BIN32_SIGNED:
+ *(Lng32 *)sec = labs(*(Lng32 *)op_data[1]);
+ if(sec < 0 )
+ {
+ ExRaiseSqlError(heap, diagsArea, EXE_BAD_ARG_TO_MATH_FUNC);
+ *(*diagsArea) << DgString0("SLEEP");
+ return ex_expr::EXPR_ERROR;
+ }
+ sleep(sec);
+ *(Int64 *)op_data[0] = 1;
+ break;
+
+ case REC_BIN64_SIGNED:
+ sec = *(Int64 *)op_data[1];
+ if(sec < 0 )
+ {
+ ExRaiseSqlError(heap, diagsArea, EXE_BAD_ARG_TO_MATH_FUNC);
+ *(*diagsArea) << DgString0("SLEEP");
+ return ex_expr::EXPR_ERROR;
+ }
+ sleep(sec);
+ *(Int64 *)op_data[0] = 1;
+ break;
+
+ default:
+ ExRaiseSqlError(heap, diagsArea, EXE_BAD_ARG_TO_MATH_FUNC);
+ *(*diagsArea) << DgString0("SLEEP");
+ return ex_expr::EXPR_ERROR;
+ break;
+ }
+ //get the seconds to sleep
+ return ex_expr::EXPR_OK;
+}
+
+ex_expr::exp_return_type ex_function_unixtime::eval(char *op_data[],
+ CollHeap* heap,
+ ComDiagsArea** diagsArea)
+{
+ char *opData = op_data[1];
+ //if there is input value
+ if(opData[0] != 0 && getNumOperands() == 2)
+ {
+ struct tm* ptr;
+ char* r = strptime(opData, "%Y-%m-%d %H:%M:%S", ptr);
+ if( r == NULL)
--- End diff --
Thanks Dave, you are correct. We want to be very strict here. I will add this checking.
---
[GitHub] trafodion pull request #1444: [TRAFODION-2954] add MySQL function unix_times...
Posted by traflm <gi...@git.apache.org>.
Github user traflm commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1444#discussion_r169511829
--- Diff: core/sql/optimizer/GroupAttr.cpp ---
@@ -1793,6 +1793,8 @@ void GroupAttributes::resolveCharacteristicInputs(const ValueIdSet& externalInpu
ItemExpr * vidExpr = vid.getItemExpr();
if ((vidExpr->getOperatorType() == ITM_CURRENT_USER) ||
(vidExpr->getOperatorType() == ITM_CURRENT_TIMESTAMP) ||
+ (vidExpr->getOperatorType() == ITM_UNIQUE_SHORT_ID) ||
+ (vidExpr->getOperatorType() == ITM_UNIQUE_ID) ||
--- End diff --
You are right, Hans, I also think in the opposite, I just simply follow the current_timestamp behavior, and this really is not what I originally want. So I will change uuid() to be evaluated once per row.
---
[GitHub] trafodion pull request #1444: [TRAFODION-2157] add MySQL function unix_times...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1444#discussion_r168606746
--- Diff: core/sql/exp/exp_function.cpp ---
@@ -2501,6 +2518,100 @@ ex_expr::exp_return_type ExFunctionReverseStr::eval(char *op_data[],
return ex_expr::EXPR_OK;
};
+ex_expr::exp_return_type ex_function_sleep::eval(char *op_data[],
+ CollHeap* heap,
+ ComDiagsArea** diagsArea)
+{
+ Int32 sec = 0;
+ switch (getOperand(1)->getDatatype())
+ {
+ case REC_BIN8_SIGNED:
+ sec = *(Int8 *)op_data[1] ;
+ if(sec < 0 )
+ {
+ ExRaiseSqlError(heap, diagsArea, EXE_BAD_ARG_TO_MATH_FUNC);
+ *(*diagsArea) << DgString0("SLEEP");
+ return ex_expr::EXPR_ERROR;
+ }
+ sleep(sec);
+ *(Int64 *)op_data[0] = 1;
+ break;
+
+ case REC_BIN16_SIGNED:
+ sec = *(short *)op_data[1] ;
+ if(sec < 0 )
+ {
+ ExRaiseSqlError(heap, diagsArea, EXE_BAD_ARG_TO_MATH_FUNC);
+ *(*diagsArea) << DgString0("SLEEP");
+ return ex_expr::EXPR_ERROR;
+ }
+ sleep(sec);
+ *(Int64 *)op_data[0] = 1;
+ break;
+
+ case REC_BIN32_SIGNED:
+ *(Lng32 *)sec = labs(*(Lng32 *)op_data[1]);
+ if(sec < 0 )
+ {
+ ExRaiseSqlError(heap, diagsArea, EXE_BAD_ARG_TO_MATH_FUNC);
+ *(*diagsArea) << DgString0("SLEEP");
+ return ex_expr::EXPR_ERROR;
+ }
+ sleep(sec);
+ *(Int64 *)op_data[0] = 1;
+ break;
+
+ case REC_BIN64_SIGNED:
+ sec = *(Int64 *)op_data[1];
+ if(sec < 0 )
+ {
+ ExRaiseSqlError(heap, diagsArea, EXE_BAD_ARG_TO_MATH_FUNC);
+ *(*diagsArea) << DgString0("SLEEP");
+ return ex_expr::EXPR_ERROR;
+ }
+ sleep(sec);
+ *(Int64 *)op_data[0] = 1;
+ break;
+
+ default:
+ ExRaiseSqlError(heap, diagsArea, EXE_BAD_ARG_TO_MATH_FUNC);
+ *(*diagsArea) << DgString0("SLEEP");
+ return ex_expr::EXPR_ERROR;
+ break;
+ }
+ //get the seconds to sleep
+ return ex_expr::EXPR_OK;
+}
+
+ex_expr::exp_return_type ex_function_unixtime::eval(char *op_data[],
+ CollHeap* heap,
+ ComDiagsArea** diagsArea)
+{
+ char *opData = op_data[1];
+ //if there is input value
+ if(opData[0] != 0 && getNumOperands() == 2)
+ {
+ struct tm* ptr;
+ char* r = strptime(opData, "%Y-%m-%d %H:%M:%S", ptr);
+ if( r == NULL)
--- End diff --
Should we also test for *r pointing to a null character? See the man page for strptime. If there is garbage data after the seconds field, r will point to the first byte of the garbage. I'm guessing we want to raise an error in that case. Consider a test of the form "if ((r == NULL) || (*r != '\0'))"
---
[GitHub] trafodion pull request #1444: [TRAFODION-2954] add MySQL function unix_times...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafodion/pull/1444
---
[GitHub] trafodion pull request #1444: [TRAFODION-2954] add MySQL function unix_times...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1444#discussion_r169392533
--- Diff: core/sql/optimizer/GroupAttr.cpp ---
@@ -1793,6 +1793,8 @@ void GroupAttributes::resolveCharacteristicInputs(const ValueIdSet& externalInpu
ItemExpr * vidExpr = vid.getItemExpr();
if ((vidExpr->getOperatorType() == ITM_CURRENT_USER) ||
(vidExpr->getOperatorType() == ITM_CURRENT_TIMESTAMP) ||
+ (vidExpr->getOperatorType() == ITM_UNIQUE_SHORT_ID) ||
+ (vidExpr->getOperatorType() == ITM_UNIQUE_ID) ||
--- End diff --
This comment applies to all the changes in this commit: Don't these code changes do the opposite of what you want to achieve? Functions like CURRENT_TIMESTAMP and CURRENT_USER are carefully designed such that we execute them only once per query, in the master. You want to make sure your functions are evaluated once per row. Did you find that these changes helped fix the issues you were seeing?
---
[GitHub] trafodion pull request #1444: [TRAFODION-2954] add MySQL function unix_times...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1444#discussion_r169390910
--- Diff: core/sql/regress/executor/TEST002 ---
@@ -1188,6 +1188,32 @@ select * from regexp_test where c1 regexp '^(25[0-5]|2[0-4][0-9]|[0-1]{1}[0-9]{2
select * from regexp_test where c1 regexp '(中文测试)';
select * from regexp_test where c1 regexp '[^\';
drop table regexp_test;
+
+--create table have 1K rows
+create table T002T1K (uniq int not null,
+ c1K int, c100 int,
+ c10 int, c1 int, c0 int )
+ STORE BY (uniq)
+ ATTRIBUTES ALIGNED FORMAT
+ SALT USING 8 PARTITIONS
+ ;
+
+upsert using load into T002T1K select
+0 + (1000 * x10) + (100 * x1) + (10 * x1) + (1 * x01),
+0 + (100 * x10) + (10 * x1) + (1 * x01),
+0 + (10 * x1) + (1 * x01),
+0 + (1 * x01),
+0,
+X01
+from (values(0)) t
+transpose 0,1,2,3,4,5,6,7,8,9 as x10
+transpose 0,1,2,3,4,5,6,7,8,9 as x1
+transpose 0,1,2,3,4,5,6,7,8,9 as X01;
+
+select sleep(5) from dual;
+select 'unixtimestamp',unix_timestamp() from dual;
+select 'uuidrow', uuid(), unix_timestamp() from T002T1K;
--- End diff --
If you want to ensure that we see different uuids and not the same one repeated, you could add a query select count(distinct uuid()) from T002T1K.
---