zhongjiajie commented on code in PR #9673:
URL: https://github.com/apache/dolphinscheduler/pull/9673#discussion_r856874109
##########
dolphinscheduler-python/pydolphinscheduler/src/pydolphinscheduler/tasks/sql.py:
##########
@@ -69,15 +70,25 @@ def __init__(
):
super().__init__(name, TaskType.SQL, *args, **kwargs)
self.sql = sql
+ self.param_sql_type = sql_type
self.datasource_name = datasource_name
self.pre_statements = pre_statements or []
self.post_statements = post_statements or []
self.display_rows = display_rows
@property
def sql_type(self) -> int:
+ """If sql type is specified by parameter"""
+ if (
+ self.param_sql_type == SqlType.SELECT
+ or self.param_sql_type == SqlType.NOT_SELECT
+ ):
+ return self.param_sql_type
Review Comment:
I think we should add some log before we retrue to user, such as
https://github.com/apache/dolphinscheduler/blob/1f48601c759df8dae82e966bed1a346416c7449a/dolphinscheduler-python/pydolphinscheduler/src/pydolphinscheduler/tasks/python.py#L81
but use `info` level
##########
dolphinscheduler-python/pydolphinscheduler/tests/tasks/test_sql.py:
##########
@@ -26,24 +26,36 @@
@pytest.mark.parametrize(
- "sql, sql_type",
+ "sql, param_sql_type, sql_type",
[
- ("select 1", SqlType.SELECT),
- (" select 1", SqlType.SELECT),
- (" select 1 ", SqlType.SELECT),
- (" select 'insert' ", SqlType.SELECT),
- (" select 'insert ' ", SqlType.SELECT),
- ("with tmp as (select 1) select * from tmp ", SqlType.SELECT),
- ("insert into table_name(col1, col2) value (val1, val2)",
SqlType.NOT_SELECT),
+ ("select 1", None, SqlType.SELECT),
+ (" select 1", None, SqlType.SELECT),
+ (" select 1 ", None, SqlType.SELECT),
+ (" select 'insert' ", None, SqlType.SELECT),
+ (" select 'insert ' ", None, SqlType.SELECT),
+ ("with tmp as (select 1) select * from tmp ", None, SqlType.SELECT),
+ (
+ "insert into table_name(col1, col2) value (val1, val2)",
+ None,
+ SqlType.NOT_SELECT,
+ ),
(
"insert into table_name(select, col2) value ('select', val2)",
+ None,
+ SqlType.NOT_SELECT,
+ ),
+ ("update table_name SET col1=val1 where col1=val2", None,
SqlType.NOT_SELECT),
+ (
+ "update table_name SET col1='select' where col1=val2",
+ None,
SqlType.NOT_SELECT,
),
- ("update table_name SET col1=val1 where col1=val2",
SqlType.NOT_SELECT),
- ("update table_name SET col1='select' where col1=val2",
SqlType.NOT_SELECT),
- ("delete from table_name where id < 10", SqlType.NOT_SELECT),
- ("delete from table_name where id < 10", SqlType.NOT_SELECT),
- ("alter table table_name add column col1 int", SqlType.NOT_SELECT),
+ ("delete from table_name where id < 10", None, SqlType.NOT_SELECT),
+ ("delete from table_name where id < 10", None, SqlType.NOT_SELECT),
+ ("alter table table_name add column col1 int", None,
SqlType.NOT_SELECT),
+ ("create table table_name2 (col1 int)", None, SqlType.NOT_SELECT),
+ ("create table table_name2 (col1 int)", SqlType.SELECT,
SqlType.SELECT),
Review Comment:
you should also add one more test case with data `("create table table_name2
(col1 int)", SqlType. NOT_SELECT, SqlType. NOT_SELECT)`
##########
dolphinscheduler-python/pydolphinscheduler/src/pydolphinscheduler/tasks/sql.py:
##########
@@ -69,15 +70,25 @@ def __init__(
):
super().__init__(name, TaskType.SQL, *args, **kwargs)
self.sql = sql
+ self.param_sql_type = sql_type
self.datasource_name = datasource_name
self.pre_statements = pre_statements or []
self.post_statements = post_statements or []
self.display_rows = display_rows
@property
def sql_type(self) -> int:
+ """If sql type is specified by parameter"""
Review Comment:
you write a wrong docstring for python, you should write in
```py
Judgement sql type, it will return the SQL type for type `SELECT` or
`NOT_SELECT`.
If parameter :param:`param_sql_type` dot not specific, will use regexp to
check
which type of the SQL is. But if parameter :param:`param_sql_type` is
specific
will use the parameter overwrites the regexp way
```
##########
dolphinscheduler-python/pydolphinscheduler/src/pydolphinscheduler/tasks/sql.py:
##########
@@ -69,15 +70,25 @@ def __init__(
):
super().__init__(name, TaskType.SQL, *args, **kwargs)
self.sql = sql
+ self.param_sql_type = sql_type
self.datasource_name = datasource_name
self.pre_statements = pre_statements or []
self.post_statements = post_statements or []
self.display_rows = display_rows
@property
def sql_type(self) -> int:
+ """If sql type is specified by parameter"""
Review Comment:
and you should remove L87
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
