Repository: aliyun/aliyun-odps-python-sdk

Issue:
在force_py 为True 时读取 VarChar(100) 列时出错
报错为 limit超过100
**报错日志:** 
```bash
 File "/opt/homebrew/Caskroom/miniconda/base/envs/py372/lib/python3.7/site-packages/odps/models/readers.py", line 81, in _retry_iter_reader
    start, record_count, step, compress=compress, columns=columns, counter=counter
  File "/opt/homebrew/Caskroom/miniconda/base/envs/py372/lib/python3.7/site-packages/odps/models/readers.py", line 143, in _open_and_iter_reader
    for record in reader[::step]:
  File "/opt/homebrew/Caskroom/miniconda/base/envs/py372/lib/python3.7/site-packages/odps/readers.py", line 115, in _iter
    record = next(self)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/py372/lib/python3.7/site-packages/odps/tunnel/io/reader.py", line 254, in __next__
    
  File "/opt/homebrew/Caskroom/miniconda/base/envs/py372/lib/python3.7/site-packages/odps/tunnel/io/reader.py", line 240, in read
    if isinstance(self._columns[0].type , types.Varchar):
  File "/opt/homebrew/Caskroom/miniconda/base/envs/py372/lib/python3.7/site-packages/odps/types.py", line 523, in __setitem__
    self._set(key, value)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/py372/lib/python3.7/site-packages/odps/types.py", line 495, in _set
    val = validate_value(value, data_type, max_field_size=self._max_field_size)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/py372/lib/python3.7/site-packages/odps/types.py", line 1618, in validate_value
    res = data_type.validate_composite_values(value)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/py372/lib/python3.7/site-packages/odps/types.py", line 1066, in validate_composite_values
    self.validate_value(value)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/py372/lib/python3.7/site-packages/odps/types.py", line 1054, in validate_value
    (len(val), self.size_limit))
ValueError: InvalidData: Length of string(153) is more than 100M.'
```
- 若通过 C 的动态库Record读取, 正常
- 走纯 python, 报错
- pyodps: `0.11.5`
- python: `python3.7/3.8/3.10`都是必现的

*反序列化之后在给对应record 的column 赋值时,没有encode转换吧?  导致valid_value报错*

Discussion:
@wjsi: Thanks for the report. Will fix in the next release.@shortwinds: varChar, decimal 都有类似的问题@wjsi: > varChar, decimal 都有类似的问题

Is there an error case for decimal type?@shortwinds:  *notes*
1. The error for the Decimal issue might have been a mistake, as retesting Decimal did not reproduce the problem.
2. A similar validate_value error for Char also exists, especially in scenarios where the character is Chinese.
**char(1) error messages**
```bash
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/models/readers.py", line 130, in __iter__
    for record in self.read():
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/models/readers.py", line 205, in read
    for record in self._open_and_iter_reader(
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/models/readers.py", line 176, in _open_and_iter_reader
    for record in reader[::step]:
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/readers.py", line 115, in _iter
    record = next(self)
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/tunnel/io/reader.py", line 386, in __next__
    record = self.read()
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/tunnel/io/reader.py", line 366, in read
    result = utils.call_with_retry(
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/utils.py", line 996, in call_with_retry
    return func(*args, **kwargs)
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/tunnel/io/reader.py", line 349, in _read_single_record
    record[i] = self._read_field(self._columns[i].type)
  File "odps/src/types_c.pyx", line 630, in odps.src.types_c.BaseRecord.__setitem__
  File "odps/src/types_c.pyx", line 576, in odps.src.types_c.BaseRecord._set
  File "odps/src/types_c.pyx", line 576, in odps.src.types_c.BaseRecord._set
  File "odps/src/types_c.pyx", line 581, in odps.src.types_c.BaseRecord._set
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/types.py", line 1825, in validate_value
    res = data_type.cast_composite_values(value)
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/types.py", line 1153, in cast_composite_values
    self.validate_value(value)
  File "/Users/admin/code/test/.venv/lib/python3.10/site-packages/odps/types.py", line 1134, in validate_value
    raise ValueError(
ValueError: InvalidData: Length of string(3) is more than 1M.'
```

**char(1)/char(3) character**
```python
# char(1)
utf_bytes1 = b'\xe9\x81\x93'

utf_str1 = utf_bytes1.decode("utf-8")
print(utf_str1)
print(len(utf_str1))

# char(3)
utf_bytes3 = b'J\xe5\x8f\xaf '
utf_str3 = utf_bytes3.decode("utf-8")
print(utf_str3)
print(len(utf_str3))
```

@wjsi BTW
```python
from odps import ODPS
o = ODPS(XXXXXXXXXXXXXX)
import sys
if 'odps.models.record' in sys.modules:
    del sys.modules['odps.models.record']
from odps import options

options.force_py = True
```
why `options.force_py = True` is not take effect, because the errors messages still use Cython `types_c.pyx`
@wjsi: @shortwinds `options.force_py` is only for test purpose. It is not recommended to use this option in production and I doubt reloading `odps.models.record` only will be effective.

Solution patch:
diff --git a/docs/source/locale/en/LC_MESSAGES/df.po b/docs/source/locale/en/LC_MESSAGES/df.po
index e340171..314d96d 100644
--- a/docs/source/locale/en/LC_MESSAGES/df.po
+++ b/docs/source/locale/en/LC_MESSAGES/df.po
@@ -36,8 +36,8 @@ msgid ""
 "PyODPS DataFrame 未来将停止维护。对于新项目，建议使用 `MaxFrame <https://"
 "maxframe.readthedocs.io/en/latest/index.html>`_\\ 。"
 msgstr ""
-"Maintenance of PyODPS DataFrame is discontinued. For new projects, please"
-" do not use this feature."
+"**Maintenance of PyODPS DataFrame is discontinued. Please do not use this"
+" feature in new projects.**"
 
 #: ../../source/df.rst:16
 msgid ""
diff --git a/odps/src/types_c.pyx b/odps/src/types_c.pyx
index 5300489..96daedc 100644
--- a/odps/src/types_c.pyx
+++ b/odps/src/types_c.pyx
@@ -93,8 +93,8 @@ cdef class StringValidator(TypeValidator):
         if s_size <= max_field_size:
             return u_val
         raise ValueError(
-            "InvalidData: Length of string(%s) is more than %sM.'" %
-            (val, max_field_size / (1024 ** 2))
+            "InvalidData: Byte length of string(%s) is more than %sM.'" %
+            (s_size, max_field_size / (1024 ** 2))
         )
 
 
@@ -118,8 +118,33 @@ cdef class BinaryValidator(TypeValidator):
         if s_size <= max_field_size:
             return bytes_val
         raise ValueError(
-            "InvalidData: Length of string(%s) is more than %sM.'" %
-            (val, max_field_size / (1024 ** 2)))
+            "InvalidData: Byte length of string(%s) is more than %sM.'" %
+            (s_size, max_field_size / (1024 ** 2)))
+
+
+cdef class SizeLimitedStringValidator(TypeValidator):
+    cdef int _size_limit
+
+    def __init__(self, int size_limit):
+        self._size_limit = size_limit
+
+    cdef object validate(self, object val, int64_t max_field_size):
+        cdef:
+            unicode u_val
+
+        if type(val) is bytes or isinstance(val, bytes):
+            u_val = (<bytes> val).decode("utf-8")
+        elif type(val) is unicode or isinstance(val, unicode):
+            u_val = <unicode> val
+        else:
+            raise TypeError("Invalid data type: expect bytes or unicode, got %s" % type(val))
+
+        if len(u_val) <= self._size_limit:
+            return u_val
+        raise ValueError(
+            "InvalidData: Length of string(%s) is more than %s.'" %
+            (val, self._size_limit)
+        )
 
 
 py_strptime = datetime.strptime
@@ -431,7 +456,9 @@ cdef object _build_type_validator(int type_id, object data_type):
     elif type_id == DOUBLE_TYPE_ID:
         return DoubleValidator()
     elif type_id == STRING_TYPE_ID:
-        if options.tunnel.string_as_binary:
+        if isinstance(data_type, types.SizeLimitedString):
+            return SizeLimitedStringValidator(data_type.size_limit)
+        elif options.tunnel.string_as_binary:
             return BinaryValidator()
         else:
             return StringValidator()
@@ -502,6 +529,8 @@ cdef class SchemaSnapshot:
 
 cdef class BaseRecord:
     def __cinit__(self, columns=None, schema=None, values=None, max_field_size=None):
+        if isinstance(columns, types.Schema):
+            schema, columns = columns, None
         self._c_schema_snapshot = getattr(schema, "_snapshot", None)
         if columns is not None:
             self._c_columns = columns
diff --git a/odps/src/utils_c.pyx b/odps/src/utils_c.pyx
index 2d17aa6..109de1c 100644
--- a/odps/src/utils_c.pyx
+++ b/odps/src/utils_c.pyx
@@ -308,4 +308,7 @@ cpdef inline unicode to_text(s, encoding="utf-8"):
 cpdef str to_lower_str(s, encoding="utf-8"):
     if s is None:
         return None
-    return to_str(s, encoding).lower()
+    if _is_py3:
+        return <str>(to_text(s, encoding).lower())
+    else:
+        return <str>(to_binary(s, encoding).lower())
diff --git a/odps/types.py b/odps/types.py
index 487972a..e9ee660 100644
--- a/odps/types.py
+++ b/odps/types.py
@@ -91,7 +91,30 @@ class Column(object):
         )
 
     def __hash__(self):
-        return hash((type(self), self.name, self.type, self.comment, self.label))
+        return hash(
+            (
+                type(self),
+                self.name,
+                self.type,
+                self.comment,
+                self.label,
+                self.nullable,
+                self.generate_expression,
+            )
+        )
+
+    def __eq__(self, other):
+        return self is other or all(
+            getattr(self, attr, None) == getattr(other, attr, None)
+            for attr in (
+                "name",
+                "type",
+                "comment",
+                "label",
+                "nullable",
+                "generate_expression",
+            )
+        )
 
     def to_sql_clause(self, with_column_comments=True):
         from .expressions import parse as parse_expression
@@ -604,6 +627,8 @@ class BaseRecord(object):
     __slots__ = "_values", "_columns", "_name_indexes", "_max_field_size"
 
     def __init__(self, columns=None, schema=None, values=None, max_field_size=None):
+        if isinstance(columns, Schema):
+            schema, columns = columns, None
         if columns is not None:
             self._columns = columns
             self._name_indexes = {
@@ -978,6 +1003,17 @@ class Double(BaseFloat):
     _type_id = 1
 
 
+def _check_string_byte_size(val, max_size):
+    if isinstance(val, six.binary_type):
+        byt_len = len(val)
+    else:
+        byt_len = 4 * len(val)
+        if byt_len > max_size:
+            # encode only when necessary
+            byt_len = len(utils.to_binary(val))
+    return byt_len <= max_size, byt_len
+
+
 @_primitive_doc(is_odps2=False)
 class String(OdpsPrimitive):
     __slots__ = ()
@@ -997,11 +1033,12 @@ class String(OdpsPrimitive):
         if val is None and self.nullable:
             return True
         max_field_size = max_field_size or self._max_length
-        if len(val) <= max_field_size:
+        valid, byt_len = _check_string_byte_size(val, max_field_size)
+        if valid:
             return True
         raise ValueError(
-            "InvalidData: Length of string(%s) is more than %sM.'"
-            % (val, max_field_size / (1024**2))
+            "InvalidData: Byte length of string(%s) is more than %sM.'"
+            % (byt_len, max_field_size / (1024**2))
         )
 
     def cast_value(self, value, data_type):
@@ -1104,11 +1141,12 @@ class Binary(OdpsPrimitive):
         if val is None and self.nullable:
             return True
         max_field_size = max_field_size or self._max_length
-        if len(val) <= max_field_size:
+        valid, byt_len = _check_string_byte_size(val, max_field_size)
+        if valid:
             return True
         raise ValueError(
-            "InvalidData: Length of binary(%s) is more than %sM.'"
-            % (val, max_field_size / (1024**2))
+            "InvalidData: Byte length of binary(%s) is more than %sM.'"
+            % (byt_len, max_field_size / (1024**2))
         )
 
     def cast_value(self, value, data_type):
@@ -1261,9 +1299,14 @@ class SizeLimitedString(String, CompositeDataType):
         if val is None and self.nullable:
             return True
         if len(val) <= self.size_limit:
+            # binary size >= unicode size
             return True
+        elif isinstance(val, six.binary_type):
+            val = val.decode("utf-8")
+            if len(val) <= self.size_limit:
+                return True
         raise ValueError(
-            "InvalidData: Length of string(%d) is more than %sM.'"
+            "InvalidData: Length of string(%d) is more than %s.'"
             % (len(val), self.size_limit)
         )
 
@@ -1734,7 +1777,7 @@ class Struct(CompositeDataType):
             isinstance(other, Struct)
             and len(self.field_types) == len(other.field_types)
             and all(
-                self.field_types[k] == other.field_types[k]
+                self.field_types[k] == other.field_types.get(k)
                 for k in six.iterkeys(self.field_types)
             )
         )


Test patch:
diff --git a/odps/tests/test_types.py b/odps/tests/test_types.py
index 89ed24d..82284ba 100644
--- a/odps/tests/test_types.py
+++ b/odps/tests/test_types.py
@@ -1,4 +1,3 @@
-#!/usr/bin/env python
 # -*- coding: utf-8 -*-
 # Copyright 1999-2025 Alibaba Group Holding Ltd.
 #
@@ -28,6 +27,7 @@ except ImportError:
 
 from .. import options
 from .. import types as odps_types
+from .. import utils
 from ..tests.core import pandas_case, py_and_c
 
 
@@ -274,19 +274,23 @@ def test_composite_types():
     comp_type = odps_types.validate_data_type("decimal(10)")
     assert isinstance(comp_type, odps_types.Decimal)
     assert comp_type.precision == 10
+    assert comp_type == "decimal(10)"
 
     comp_type = odps_types.validate_data_type("decimal(10, 2)")
     assert isinstance(comp_type, odps_types.Decimal)
     assert comp_type.precision == 10
     assert comp_type.scale == 2
+    assert comp_type == "decimal(10,2)"
 
     comp_type = odps_types.validate_data_type("varchar(10)")
     assert isinstance(comp_type, odps_types.Varchar)
     assert comp_type.size_limit == 10
+    assert comp_type == "varchar(10)"
 
     comp_type = odps_types.validate_data_type("char(20)")
     assert isinstance(comp_type, odps_types.Char)
     assert comp_type.size_limit == 20
+    assert comp_type == "char(20)"
 
     with pytest.raises(ValueError) as ex_info:
         odps_types.validate_data_type("array")
@@ -295,6 +299,7 @@ def test_composite_types():
     comp_type = odps_types.validate_data_type("array<bigint>")
     assert isinstance(comp_type, odps_types.Array)
     assert isinstance(comp_type.value_type, odps_types.Bigint)
+    assert comp_type == "array<bigint>"
 
     with pytest.raises(ValueError) as ex_info:
         odps_types.validate_data_type("map")
@@ -304,12 +309,16 @@ def test_composite_types():
     assert isinstance(comp_type, odps_types.Map)
     assert isinstance(comp_type.key_type, odps_types.Bigint)
     assert isinstance(comp_type.value_type, odps_types.String)
+    assert comp_type == "map<bigint, string>"
 
     comp_type = odps_types.validate_data_type("struct<abc:int, def:string>")
     assert isinstance(comp_type, odps_types.Struct)
     assert len(comp_type.field_types) == 2
     assert isinstance(comp_type.field_types["abc"], odps_types.Int)
     assert isinstance(comp_type.field_types["def"], odps_types.String)
+    assert comp_type == "struct<abc:int, def:string>"
+    assert comp_type != "struct<abc:int>"
+    assert comp_type != "struct<abc:int, uvw:string>"
 
     comp_type = odps_types.validate_data_type(
         "struct<abc:int, def:map<bigint, string>, ghi:string>"
@@ -347,10 +356,15 @@ def test_set_with_cast():
 @py_and_c_deco
 def test_record_copy():
     s = TableSchema.from_lists(["col1"], ["string"])
-    r = Record(schema=s)
+    r = Record(s)
     r.col1 = "a"
 
     cr = copy.copy(r)
+    assert cr == r
+    assert cr.col1 == r.col1
+
+    cr = copy.deepcopy(r)
+    assert cr == r
     assert cr.col1 == r.col1
 
 
@@ -515,6 +529,42 @@ def test_validate_struct():
         options.struct_as_dict = False
 
 
+@py_and_c_deco
+@pytest.mark.parametrize("use_binary", [False, True])
+def test_varchar_size_limit(use_binary):
+    def _c(s):
+        if use_binary:
+            return utils.to_binary(s)
+        return utils.to_text(s)
+
+    s = TableSchema.from_lists(["col1"], ["varchar(3)"])
+    r = Record(schema=s)
+    r[0] = _c("123")
+    r[0] = _c("测试字")
+    with pytest.raises(ValueError):
+        r[0] = _c("1234")
+    with pytest.raises(ValueError):
+        r[0] = _c("测试字符")
+
+
+@py_and_c_deco
+@pytest.mark.parametrize("use_binary", [False, True])
+def test_field_size_limit(use_binary):
+    def _c(s):
+        if use_binary:
+            return utils.to_binary(s)
+        return utils.to_text(s)
+
+    s = TableSchema.from_lists(["str_col", "bin_col"], ["string", "binary"])
+    r = Record(schema=s, max_field_size=1024)
+    r[0] = _c("1" * 1024)
+    r[1] = _c("1" * 1024)
+    with pytest.raises(ValueError):
+        r[0] = _c("1" * 1023 + "测")
+    with pytest.raises(ValueError):
+        r[1] = _c("1" * 1023 + "测")
+
+
 def test_validate_decimal():
     with pytest.raises(ValueError):
         odps_types.Decimal(32, 60)