Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add timediff function #1505

Merged
merged 2 commits into from
Jul 23, 2019
Merged

Add timediff function #1505

merged 2 commits into from
Jul 23, 2019

Conversation

HangyuanLiu
Copy link
Contributor

@HangyuanLiu HangyuanLiu commented Jul 18, 2019

Add timediff function

#1428

@chenhao7253886
Copy link
Contributor

I think that you need't import a new udf type to implement timediff.

@@ -552,6 +554,7 @@
'VARCHAR': 'string_val',
'DATE': 'datetime_val',
'DATETIME': 'datetime_val',
'TIME': 'datetime_val',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to use bigint_val

@@ -89,6 +91,11 @@ public IntLiteral(long longValue, Type type) throws AnalysisException {
}
// no need to check upper bound
break;
case TIME:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is not fit to add type TIME in IntLiteral

@@ -105,7 +105,8 @@ Status ResultWriter::add_one_row(TupleRow* row) {
break;

case TYPE_DATE:
case TYPE_DATETIME: {
case TYPE_DATETIME:
case TYPE_TIME: {
char buf[64];
const DateTimeValue* time_val = (const DateTimeValue*)(item);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think use bigint to represent time in memory?

@HangyuanLiu HangyuanLiu force-pushed the add-timediff branch 2 times, most recently from 403f208 to 4fb06b4 Compare July 22, 2019 03:17
@@ -87,6 +87,11 @@ Literal::Literal(const TExprNode& node) :
_value.datetime_val.from_date_str(
node.date_literal.value.c_str(), node.date_literal.value.size());
break;
case TYPE_TIME:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reuse TYPE_DOUBLE

public:
static void init();

static BooleanVal cast_to_boolean_val(FunctionContext*, const DoubleVal&);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can only support time to double and to string

@@ -267,6 +267,7 @@ void RawValue::write(const void* value, void* dst, const TypeDescriptor& type, M
break;
}

case TYPE_TIME:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be the same as TYPE_DOUBLE

@@ -104,6 +104,39 @@ Status ResultWriter::add_one_row(TupleRow* row) {
buf_ret = _row_buffer->push_double(*static_cast<double*>(item));
break;

case TYPE_TIME: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can write a function to do this to avoid write this many times

@@ -480,6 +481,7 @@ public static String getUdfType(PrimitiveType t) {
case INT:
return "IntVal";
case BIGINT:
case TIME:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why BIGINT?

@@ -480,6 +486,7 @@ public static boolean isImplicitCast(PrimitiveType type, PrimitiveType target) {
compatibilityMatrix[DECIMALV2.ordinal()][DECIMAL.ordinal()] = DECIMALV2;

compatibilityMatrix[HLL.ordinal()][HLL.ordinal()] = HLL;
compatibilityMatrix[TIME.ordinal()][TIME.ordinal()] = TIME;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should add more relation between time and other types

@HangyuanLiu HangyuanLiu force-pushed the add-timediff branch 7 times, most recently from e649d62 to 92d7b85 Compare July 23, 2019 03:56
Copy link
Contributor

@imay imay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@imay imay merged commit 4aedaea into apache:master Jul 23, 2019
morningman pushed a commit to baidu-doris/incubator-doris that referenced this pull request Jul 28, 2019
morningman pushed a commit to baidu-doris/incubator-doris that referenced this pull request Jul 28, 2019
@imay imay mentioned this pull request Sep 26, 2019
@HangyuanLiu HangyuanLiu deleted the add-timediff branch December 25, 2019 02:06
luwei16 pushed a commit to luwei16/incubator-doris that referenced this pull request Apr 7, 2023
SWJTU-ZhangLei pushed a commit to SWJTU-ZhangLei/incubator-doris that referenced this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants