Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
mchades merged PR #5646: URL: https://github.com/apache/gravitino/pull/5646 -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
tengqm commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2517695461 lgtm -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2517314980 hi @tengqm :-) I revert `remove List` commit, Please help me review it again, Thanks! -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
tengqm commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2516995520 Just check the github workflow for this project: https://github.com/apache/gravitino/blob/main/.github/workflows/python-integration-test.yml#L76 Looks like we are testing against 3.8, 3.9, 3.10, 3.11. -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
tengqm commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2516941445 > hi @tengqm Gravitino Python client went to support from 3.7 to 3.10, So maybe we must use `typing.List` type to support low Python version. In that case, we have to stick to the old way. 3.7-3.10 is a large range anyway. -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2516778039 hi @tengqm Gravitino Python client went to support from 3.7 to 3.10, So maybe we must use `typing.List` type to support low Python version. -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2516721369 When I use `list` to replace `List`. The `TestLiterals` throw these errors. ``` The error TypeError: 'type' object is not subscriptable occurs because the list type is not subscriptable in Python versions earlier than 3.9. To fix this, you should use List from the typing module instead of list. ``` @tengqm @noidname01 Do you have any suggestions to fix this problem? I rollback this change?? -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on code in PR #5646:
URL: https://github.com/apache/gravitino/pull/5646#discussion_r1869043083
##
clients/client-python/gravitino/api/expressions/literals/literals.py:
##
@@ -0,0 +1,137 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import decimal
+from typing import TypeVar
+from datetime import date, time, datetime
+
+from gravitino.api.expressions.literals.literal import Literal
+from gravitino.api.types.type import Type
+from gravitino.api.types.types import Types
+
+T = TypeVar("T")
+
+
+class LiteralImpl(Literal[T]):
+"""Creates a literal with the given type value."""
+
+_value: T
+_data_type: Type
+
+def __init__(self, value: T, data_type: Type):
+self._value = value
+self._data_type = data_type
+
+def value(self) -> T:
+return self._value
+
+def data_type(self) -> Type:
+return self._data_type
+
+def __eq__(self, other: object) -> bool:
+if not isinstance(other, LiteralImpl):
+return False
+return (self._value == other._value) and (self._data_type ==
other._data_type)
+
+def __hash__(self):
+return hash((self._value, self._data_type))
+
+def __str__(self):
+return f"LiteralImpl(value={self._value}, data_type={self._data_type})"
+
+
+class Literals:
Review Comment:
hi @tengqm I know what you mean.
We also refer to Iceberg's Python client code style.
https://github.com/apache/iceberg-python/blob/main/pyiceberg/table/name_mapping.py
At present, there are not enough experienced Python engineers in the
community, which is the result of the best efforts of the community
contributors.
We keep the python client code consistent with the java code to ensure that
the functionality is consistent.
So I think we didn't modify these codes.
--
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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on code in PR #5646:
URL: https://github.com/apache/gravitino/pull/5646#discussion_r1869043083
##
clients/client-python/gravitino/api/expressions/literals/literals.py:
##
@@ -0,0 +1,137 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import decimal
+from typing import TypeVar
+from datetime import date, time, datetime
+
+from gravitino.api.expressions.literals.literal import Literal
+from gravitino.api.types.type import Type
+from gravitino.api.types.types import Types
+
+T = TypeVar("T")
+
+
+class LiteralImpl(Literal[T]):
+"""Creates a literal with the given type value."""
+
+_value: T
+_data_type: Type
+
+def __init__(self, value: T, data_type: Type):
+self._value = value
+self._data_type = data_type
+
+def value(self) -> T:
+return self._value
+
+def data_type(self) -> Type:
+return self._data_type
+
+def __eq__(self, other: object) -> bool:
+if not isinstance(other, LiteralImpl):
+return False
+return (self._value == other._value) and (self._data_type ==
other._data_type)
+
+def __hash__(self):
+return hash((self._value, self._data_type))
+
+def __str__(self):
+return f"LiteralImpl(value={self._value}, data_type={self._data_type})"
+
+
+class Literals:
Review Comment:
hi @tengqm I know what you mean.
We also refer to Iceberg's Python client code style.
https://github.com/apache/iceberg-python/blob/main/pyiceberg/table/name_mapping.py
At present, there are not enough experienced Python engineers in the
community, which is the result of the best efforts of the community partners.
We keep the python client code consistent with the java code to ensure that
the functionality is consistent.
So I think we didn't modify these codes.
--
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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
tengqm commented on code in PR #5646:
URL: https://github.com/apache/gravitino/pull/5646#discussion_r1868979622
##
clients/client-python/gravitino/api/expressions/literals/literals.py:
##
@@ -0,0 +1,137 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import decimal
+from typing import TypeVar
+from datetime import date, time, datetime
+
+from gravitino.api.expressions.literals.literal import Literal
+from gravitino.api.types.type import Type
+from gravitino.api.types.types import Types
+
+T = TypeVar("T")
+
+
+class LiteralImpl(Literal[T]):
+"""Creates a literal with the given type value."""
+
+_value: T
+_data_type: Type
+
+def __init__(self, value: T, data_type: Type):
+self._value = value
+self._data_type = data_type
+
+def value(self) -> T:
+return self._value
+
+def data_type(self) -> Type:
+return self._data_type
+
+def __eq__(self, other: object) -> bool:
+if not isinstance(other, LiteralImpl):
+return False
+return (self._value == other._value) and (self._data_type ==
other._data_type)
+
+def __hash__(self):
+return hash((self._value, self._data_type))
+
+def __str__(self):
+return f"LiteralImpl(value={self._value}, data_type={self._data_type})"
+
+
+class Literals:
Review Comment:
Since we already have a 'Literal' class in another file,
defining a new class called 'Literals' may confuse other developers/users.
##
clients/client-python/gravitino/api/expressions/literals/literal.py:
##
@@ -0,0 +1,43 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from abc import abstractmethod
+from typing import List, TypeVar, Generic
Review Comment:
You may want to kill 'List' here.
##
clients/client-python/gravitino/api/expressions/literals/literals.py:
##
@@ -0,0 +1,137 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import decimal
+from typing import TypeVar
+from datetime import date, time, datetime
+
+from gravitino.api.expressions.literals.literal import Literal
+from gravitino.api.types.type import Type
+from gravitino.api.types.types import Types
+
+T = TypeVar("T")
+
+
+class LiteralImpl(Literal[T]):
+"""Creates a literal with the given type value."""
+
+_value: T
+_data_type: Type
+
+def __init__(self, value: T, data_type: Type):
+self._value = value
+self._data_type = data_type
+
+def value(self) -> T:
+return self._value
+
+def data_type(self) -> Type:
+return self._data_type
+
+def __eq__(self, other: object) -> bool:
+if not isinstance(other, LiteralImpl):
+return False
+return (self._value == other._va
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
tengqm commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1868974650 ## clients/client-python/gravitino/api/types/types.py: ## @@ -123,7 +123,7 @@ def get(cls) -> "ShortType": return cls(True) @classmethod -def unsigned(cls): +def unsigned(cls) -> "ShortType": Review Comment: Em... I am not a big fan of passing lint checking simply for passing's purpose. At the end of the day, we are the maintainers to maintain these code. If lint rules are not appropriate in our context, we may want to disable that particular rule. It is not an uncommon practice. At the same time, if we are forced to "hack" the code to make the linter happy, we may need to revise our design. Probably there are more straightforward ways to achieve the same end. -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2516471591 hi @tengqm We are improving PR base on your comments, Please help me review it again, thanks! -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1868793998 ## clients/client-python/gravitino/api/expressions/function_expression.py: ## @@ -0,0 +1,95 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations +from abc import abstractmethod +from typing import List + +from gravitino.api.expressions.expression import Expression + + +class FunctionExpression(Expression): +""" +The interface of a function expression. A function expression is an expression that takes a +function name and a list of arguments. +""" + +@staticmethod +def of(function_name: str, *arguments: Expression) -> FuncExpressionImpl: +""" +Creates a new FunctionExpression with the given function name. +If no arguments are provided, it uses an empty expression. + +:param function_name: The name of the function. +:param arguments: The arguments to the function (optional). +:return: The created FunctionExpression. +""" +arguments = list(arguments) if arguments else Expression.EMPTY_EXPRESSION +return FuncExpressionImpl(function_name, arguments) + +@abstractmethod +def function_name(self) -> str: +"""Returns the function name.""" +pass Review Comment: Yes, You are right, I will fix this problem. ## clients/client-python/gravitino/api/expressions/function_expression.py: ## @@ -0,0 +1,95 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations +from abc import abstractmethod +from typing import List + +from gravitino.api.expressions.expression import Expression + + +class FunctionExpression(Expression): +""" +The interface of a function expression. A function expression is an expression that takes a +function name and a list of arguments. +""" + +@staticmethod +def of(function_name: str, *arguments: Expression) -> FuncExpressionImpl: +""" +Creates a new FunctionExpression with the given function name. +If no arguments are provided, it uses an empty expression. + +:param function_name: The name of the function. +:param arguments: The arguments to the function (optional). +:return: The created FunctionExpression. +""" +arguments = list(arguments) if arguments else Expression.EMPTY_EXPRESSION +return FuncExpressionImpl(function_name, arguments) + +@abstractmethod +def function_name(self) -> str: +"""Returns the function name.""" +pass + +@abstractmethod +def arguments(self) -> List[Expression]: +"""Returns the arguments passed to the function.""" +pass + +def children(self) -> List[Expression]: +"""Returns the arguments as children.""" +return self.arguments() + + +class FuncExpressionImpl(FunctionExpression): +""" +A concrete implementation of the FunctionExpression interface. +""" + +_function_name: str Review Comment: > for a member that is well scoped, I don't think we need a sophisticated name. > I'd suggest we use _name as the property name rather than _function_name. and > similarly, given an instance impl of class FuncExpressionImpl, > the following calls are still pretty clear without the complex names: hi @tengqm Will want to keep Python code consistent with Java code, ht
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
SophieTech88 commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2516323555 The Integration Test failure is related to the List typing things in literal.py file. -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
SophieTech88 commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1868774787 ## clients/client-python/gravitino/api/types/types.py: ## @@ -123,7 +123,7 @@ def get(cls) -> "ShortType": return cls(True) @classmethod -def unsigned(cls): +def unsigned(cls) -> "ShortType": Review Comment: If we remove the quote, there is a Pylance errors, and it won't pass the pylint check. -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
SophieTech88 commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1868770901 ## clients/client-python/gravitino/api/types/types.py: ## @@ -123,7 +123,7 @@ def get(cls) -> "ShortType": return cls(True) @classmethod -def unsigned(cls): +def unsigned(cls) -> "ShortType": Review Comment: ``` class ShortType(IntegralType): _instance: "ShortType" = None _unsigned_instance: "ShortType" = None ``` The type annotation ShortType is quoted because `_instance` is an attribute of the class that references ShortType, and the class itself isn't fully defined at the time this attribute is declared. The return type annotations (-> "ShortType") are also quoted for the same reason: `ShortType` isn't fully defined when these methods are declared. -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
SophieTech88 commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1868747950 ## clients/client-python/gravitino/api/expressions/expression.py: ## @@ -0,0 +1,51 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations +from abc import ABC, abstractmethod +from typing import List, Set, TYPE_CHECKING Review Comment: Yeah. List, Set are deprecated from typing in Python 3.9. We can use list and set directly. -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
tengqm commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1868687438 ## clients/client-python/gravitino/api/expressions/function_expression.py: ## @@ -0,0 +1,95 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations +from abc import abstractmethod +from typing import List + +from gravitino.api.expressions.expression import Expression + + +class FunctionExpression(Expression): +""" +The interface of a function expression. A function expression is an expression that takes a +function name and a list of arguments. +""" + +@staticmethod +def of(function_name: str, *arguments: Expression) -> FuncExpressionImpl: +""" +Creates a new FunctionExpression with the given function name. +If no arguments are provided, it uses an empty expression. + +:param function_name: The name of the function. +:param arguments: The arguments to the function (optional). +:return: The created FunctionExpression. +""" +arguments = list(arguments) if arguments else Expression.EMPTY_EXPRESSION +return FuncExpressionImpl(function_name, arguments) + +@abstractmethod +def function_name(self) -> str: +"""Returns the function name.""" +pass Review Comment: As noted previously, please reconsider all these `pass` cases ... do we really want to silently succeed here? ## clients/client-python/gravitino/api/types/types.py: ## @@ -123,7 +123,7 @@ def get(cls) -> "ShortType": return cls(True) @classmethod -def unsigned(cls): +def unsigned(cls) -> "ShortType": Review Comment: Can you help share with me what this syntax is? I mean ... why the return type is quoted? ## clients/client-python/gravitino/api/expressions/function_expression.py: ## @@ -0,0 +1,95 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations +from abc import abstractmethod +from typing import List + +from gravitino.api.expressions.expression import Expression + + +class FunctionExpression(Expression): +""" +The interface of a function expression. A function expression is an expression that takes a +function name and a list of arguments. +""" + +@staticmethod +def of(function_name: str, *arguments: Expression) -> FuncExpressionImpl: +""" +Creates a new FunctionExpression with the given function name. +If no arguments are provided, it uses an empty expression. + +:param function_name: The name of the function. +:param arguments: The arguments to the function (optional). +:return: The created FunctionExpression. +""" +arguments = list(arguments) if arguments else Expression.EMPTY_EXPRESSION +return FuncExpressionImpl(function_name, arguments) + +@abstractmethod +def function_name(self) -> str: +"""Returns the function name.""" +pass + +@abstractmethod +def arguments(self) -> List[Expression]: +"""Returns the arguments passed to the function.""" +pass + +def children(self) -> List[Expression]: +"""Returns the arguments as children.""" +return self.arguments() + + +class FuncExpressionImpl(FunctionExpression): +""" +A concrete implementation of the FunctionExpression interface. +""" + +_func
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2516025721 hi @tengqm Do you want to have another look? -- 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]
Re: [PR] [#5201] feat(client-python): Implement expressions in python client [gravitino]
jerryshao commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2513930470 LGTM. -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1867204176 ## clients/client-python/gravitino/api/types/decimal.py: ## Review Comment: @mchades I removed `Decimal` type. Please help me review again, Thanks. -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
mchades commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1865740827 ## clients/client-python/gravitino/api/types/decimal.py: ## Review Comment: this is unnecessary since https://github.com/apache/gravitino/pull/5354#discussion_r1828647646 -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
noidname01 commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1865641877 ## clients/client-python/gravitino/api/expressions/expression.py: ## @@ -0,0 +1,51 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations +from abc import ABC, abstractmethod +from typing import List, Set, TYPE_CHECKING + +if TYPE_CHECKING: +from gravitino.api.expressions.named_reference import NamedReference + + +class Expression(ABC): +"""Base class of the public logical expression API.""" + +EMPTY_EXPRESSION: List[Expression] = [] Review Comment: Sure, then overall LGTM! -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1865637817 ## clients/client-python/gravitino/api/expressions/expression.py: ## @@ -0,0 +1,51 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations +from abc import ABC, abstractmethod +from typing import List, Set, TYPE_CHECKING + +if TYPE_CHECKING: +from gravitino.api.expressions.named_reference import NamedReference + + +class Expression(ABC): +"""Base class of the public logical expression API.""" + +EMPTY_EXPRESSION: List[Expression] = [] Review Comment: hi @noidname01 The `EMPTY_EXPRESSION` value does not fill in any data. So, I think we didn't need to modify this code, what's do you think? -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
noidname01 commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1865583011 ## clients/client-python/gravitino/api/expressions/expression.py: ## @@ -0,0 +1,51 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations +from abc import ABC, abstractmethod +from typing import List, Set, TYPE_CHECKING + +if TYPE_CHECKING: +from gravitino.api.expressions.named_reference import NamedReference + + +class Expression(ABC): +"""Base class of the public logical expression API.""" + +EMPTY_EXPRESSION: List[Expression] = [] Review Comment: This might cause unexpected behaviour if it will be modified at any moment (like append, delete, e.g.), because this list would share between classes, any modification would affect where it exists once and for all at the same time. Unless we're sure that the scenario above would not happen, I think it's better to use immutable object here or some other methods. -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
noidname01 commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2511083282 Overall LGTM! -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
SophieTech88 commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1865274947 ## clients/client-python/gravitino/api/expressions/literals/literals.py: ## @@ -0,0 +1,149 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from decimal import Decimal +from typing import Union +from datetime import date, time, datetime + +from gravitino.api.expressions.literals.literal import Literal +from gravitino.api.types import Types + + +class LiteralImpl(Literal): +"""Creates a literal with the given type value.""" + +_value: Union[int, float, Decimal, str, date, time, datetime, bool, None] +_data_type: Union[ Review Comment: The changes for the types things, LGTM. Many thanks! -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1865222574 ## clients/client-python/gravitino/api/expressions/literals/literals.py: ## @@ -0,0 +1,149 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from decimal import Decimal +from typing import Union +from datetime import date, time, datetime + +from gravitino.api.expressions.literals.literal import Literal +from gravitino.api.types import Types + + +class LiteralImpl(Literal): +"""Creates a literal with the given type value.""" + +_value: Union[int, float, Decimal, str, date, time, datetime, bool, None] +_data_type: Union[ Review Comment: I improve this part codes @SophieTech88 @noidname01 Please help me review, thanks. -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
mchades commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1859651030 ## clients/client-python/gravitino/api/expressions/literals/literals.py: ## @@ -0,0 +1,149 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from decimal import Decimal +from typing import Union +from datetime import date, time, datetime + +from gravitino.api.expressions.literals.literal import Literal +from gravitino.api.types import Types + + +class LiteralImpl(Literal): +"""Creates a literal with the given type value.""" + +_value: Union[int, float, Decimal, str, date, time, datetime, bool, None] +_data_type: Union[ Review Comment: seems should be `_data_type: Type`? @xunliu cc -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
SophieTech88 commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1859606707 ## clients/client-python/gravitino/api/expressions/literals/literals.py: ## @@ -0,0 +1,138 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from decimal import Decimal +from typing import Union +from datetime import date, time, datetime + +from gravitino.api.expressions.literals.literal import Literal + + +class LiteralImpl(Literal): +"""Creates a literal with the given type value.""" + +_value: Union[int, float, str, datetime, time, date, bool, Decimal, None] +_data_type: ( +str # TODO: Need implement `api/src/main/java/org/apache/gravitino/rel/types` Review Comment: I just update the _data_type in the new commit -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
SophieTech88 commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1858812314 ## clients/client-python/gravitino/api/expressions/literals/literals.py: ## @@ -0,0 +1,138 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from decimal import Decimal +from typing import Union +from datetime import date, time, datetime + +from gravitino.api.expressions.literals.literal import Literal + + +class LiteralImpl(Literal): +"""Creates a literal with the given type value.""" + +_value: Union[int, float, str, datetime, time, date, bool, Decimal, None] +_data_type: ( +str # TODO: Need implement `api/src/main/java/org/apache/gravitino/rel/types` Review Comment: Oh. Yeah. Do I need to import and list all types in this _data_type? -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
mchades commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1858734503 ## clients/client-python/gravitino/api/expressions/literals/literals.py: ## @@ -0,0 +1,138 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from decimal import Decimal +from typing import Union +from datetime import date, time, datetime + +from gravitino.api.expressions.literals.literal import Literal + + +class LiteralImpl(Literal): +"""Creates a literal with the given type value.""" + +_value: Union[int, float, str, datetime, time, date, bool, Decimal, None] +_data_type: ( +str # TODO: Need implement `api/src/main/java/org/apache/gravitino/rel/types` Review Comment: you already implemented types `clients/client-python/gravitino/api/types.py` and type `clients/client-python/gravitino/api/type.py` -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
SophieTech88 commented on PR #5646: URL: https://github.com/apache/gravitino/pull/5646#issuecomment-2501017886 > > I'd suggest we split this into a few smaller PRs. > > Agree with @tengqm, the current PR is too large to review. > > As this PR is focused on expressions, I suggest moving `distributions`, `sorts`, and `transforms` to separate PRs. This will make it easier for us to review this PR. WDYT? @SophieTech88 Of course. Just update the PR, removed `distributions`, `sorts`, and `transforms` . -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
tengqm commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1858486936 ## clients/client-python/gravitino/api/expressions/Literals/literals.py: ## @@ -0,0 +1,138 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from decimal import Decimal +from typing import Union +from datetime import date, time, datetime + +from gravitino.api.expressions.literals.literal import Literal + + +class LiteralImpl(Literal): +"""Creates a literal with the given type value.""" + +_value: Union[int, float, str, datetime, time, date, bool, Decimal, None] +_data_type: ( +str # TODO: Need implement `api/src/main/java/org/apache/gravitino/rel/types` +) + +def __init__( +self, +value: Union[int, float, str, datetime, time, date, bool, Decimal, None], Review Comment: Got it. -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
tengqm commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1858485999 ## clients/client-python/gravitino/api/expressions/named_reference.py: ## @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one Review Comment: Really appreciate this clarification. Thanks. -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
mchades commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1857896650 ## clients/client-python/gravitino/api/expressions/named_reference.py: ## @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one Review Comment: > Why do we call this a named reference? It represents a reference to a field/column by its name, most common way to refer to columns in SQL: `SELECT name FROM table` > Is there a case where a reference has no name? Yes, examples include: - Positional references: `SELECT $1` - Expression results: `SELECT (a + b)` - Anonymous subquery columns > Do we have to differentiate these two types of references? We currently don't have `UnnamedReference` in Gravitino because we haven't encountered the usage scenarios. So my suggestion is to keep it as it is like Java implementation. -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1857729391 ## clients/client-python/gravitino/api/expressions/named_reference.py: ## @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one Review Comment: @mchades Do you have any suggestions? -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1857726745 ## clients/client-python/gravitino/api/expressions/named_reference.py: ## @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one Review Comment: hi @tengqm because we need to implement all classes in the Python client. - https://github.com/apache/gravitino/blob/main/api/src/main/java/org/apache/gravitino/rel/expressions/NamedReference.java -- 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]
Re: [PR] [5201] feat(client-python): Implement expressions in python client [gravitino]
xunliu commented on code in PR #5646: URL: https://github.com/apache/gravitino/pull/5646#discussion_r1857728991 ## clients/client-python/gravitino/api/expressions/Literals/literals.py: ## @@ -0,0 +1,138 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from decimal import Decimal +from typing import Union +from datetime import date, time, datetime + +from gravitino.api.expressions.literals.literal import Literal + + +class LiteralImpl(Literal): +"""Creates a literal with the given type value.""" + +_value: Union[int, float, str, datetime, time, date, bool, Decimal, None] +_data_type: ( +str # TODO: Need implement `api/src/main/java/org/apache/gravitino/rel/types` +) + +def __init__( +self, +value: Union[int, float, str, datetime, time, date, bool, Decimal, None], Review Comment: Because we need to support `Decimal` type here. - https://github.com/apache/gravitino/blob/main/api/src/main/java/org/apache/gravitino/rel/expressions/literals/Literals.java#L134 -- 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]
