jt2594838 commented on code in PR #773:
URL: https://github.com/apache/tsfile/pull/773#discussion_r3068002318
##########
python/tsfile/dataset/dataframe.py:
##########
@@ -557,14 +576,31 @@ def _load_metadata(self):
if not self._index.series_refs_ordered:
raise ValueError("No valid time series found in the provided
TsFile files")
+ def _show_loading_progress(self, done: int, total: int, total_series: int
= None):
+ if not self._show_progress or total <= 0:
+ return
+
+ if total_series is None:
+ sys.stderr.write(f"\rLoading TsFile shards: {done}/{total}")
Review Comment:
Is it proper to report progress in stderr?
##########
python/tests/test_tsfile_dataset.py:
##########
@@ -474,3 +723,182 @@ def
test_series_path_resolution_allows_prefix_tag_values():
series_path = build_series_path(catalog, device_id, 0)
assert series_path == "weather.site_a.device_a.temperature"
assert resolve_series_path(catalog, series_path) == (table_id, device_id,
0)
+
+
+def test_series_path_resolution_allows_missing_trailing_tag_value():
+ catalog = MetadataCatalog()
+ table_id = catalog.add_table(
+ "weather",
+ ("device",),
+ (TSDataType.STRING,),
+ ("temperature",),
+ )
+ device_id = catalog.add_device(table_id, (), 0, 1)
+ catalog.series_stats_by_ref[(device_id, 0)] = {
+ "length": 1,
+ "min_time": 0,
+ "max_time": 0,
+ "timeline_length": 1,
+ "timeline_min_time": 0,
+ "timeline_max_time": 0,
+ }
+
+ series_path = build_series_path(catalog, device_id, 0)
+ assert series_path == "weather.temperature"
+ assert resolve_series_path(catalog, series_path) == (table_id, device_id,
0)
+
+
+def test_series_path_resolution_uses_named_tags_for_sparse_non_prefix_values():
+ catalog = MetadataCatalog()
+ table_id = catalog.add_table(
+ "weather",
+ ("city", "device", "region"),
+ (TSDataType.STRING, TSDataType.STRING, TSDataType.STRING),
+ ("temperature",),
+ )
+ device_id = catalog.add_device(table_id, (None, "device_a", None), 0, 1)
+ catalog.series_stats_by_ref[(device_id, 0)] = {
+ "length": 1,
+ "min_time": 0,
+ "max_time": 0,
+ "timeline_length": 1,
+ "timeline_min_time": 0,
+ "timeline_max_time": 0,
+ }
+
+ series_path = build_series_path(catalog, device_id, 0)
+ assert series_path == "weather.device_a.temperature"
Review Comment:
how could you distinguish between (none,devicea,none) ,(devicea,none,none)?
##########
python/tsfile/dataset/reader.py:
##########
@@ -47,6 +47,8 @@ def _to_python_scalar(value):
def _build_exact_tag_filter(tag_values: Dict[str, object]):
tag_filter = None
for tag_column, tag_value in tag_values.items():
+ if tag_value is None:
+ continue
Review Comment:
why should none be skipped?
if the provided tag values are (a, none,c)
then (a,b,c)should not be matched
##########
python/tsfile/dataset/metadata.py:
##########
@@ -187,25 +230,45 @@ def resolve_series_path(catalog: MetadataCatalog,
series_path: str) -> Tuple[int
table_id = catalog.table_id_by_name[table_name]
table_entry = catalog.table_entries[table_id]
- expected_parts = len(table_entry.tag_columns) + 2
- if len(parts) > expected_parts:
- raise ValueError(f"Series not found: {series_path}")
-
field_name = parts[-1]
try:
field_idx = table_entry.get_field_index(field_name)
except ValueError as exc:
raise ValueError(f"Series not found: {series_path}") from exc
- tag_values = tuple(
+ tag_parts = parts[1:-1]
+ direct_device_id = None
+ direct_tag_values = _normalize_tag_values(
_coerce_path_component(raw_value, tag_type)
- for raw_value, tag_type in zip(parts[1:-1], table_entry.tag_types)
+ for raw_value, tag_type in zip(tag_parts, table_entry.tag_types)
)
- key = (table_id, tag_values)
- if key not in catalog.device_id_by_key:
+ direct_key = (table_id, direct_tag_values)
+ if direct_key in catalog.device_id_by_key:
+ direct_device_id = catalog.device_id_by_key[direct_key]
+
+ if table_id not in catalog.tables_with_sparse_tag_values:
+ if direct_device_id is None:
+ raise ValueError(f"Series not found: {series_path}")
+ return table_id, direct_device_id, field_idx
+
+ compressed_key = (table_id, tuple(tag_parts))
+ sparse_device_ids =
catalog.sparse_device_ids_by_compressed_path.get(compressed_key, [])
+ candidate_ids = []
+ seen_ids = set()
Review Comment:
is it enough to use seen_ids only?
##########
python/tsfile/dataset/metadata.py:
##########
@@ -148,17 +170,38 @@ def split_logical_series_path(series_path: str) ->
List[str]:
return parts
-def build_logical_series_path(table_name: str, tag_values: Iterable[Any],
field_name: str) -> str:
- components = [table_name, *tag_values, field_name]
+def build_logical_series_path(
+ table_name: str,
+ tag_values: Iterable[Any],
+ field_name: str,
+ tag_columns: Iterable[str] = (),
+) -> str:
+ components = build_logical_series_components(table_name, tag_values,
field_name, tag_columns)
return _PATH_SEPARATOR.join(_escape_path_component(component) for
component in components)
+def build_logical_series_components(
+ table_name: str,
+ tag_values: Iterable[Any],
+ field_name: str,
+ tag_columns: Iterable[str] = (),
+) -> List[str]:
+ del tag_columns
Review Comment:
what is this
##########
python/tsfile/dataset/dataframe.py:
##########
@@ -664,11 +705,26 @@ def __len__(self) -> int:
return len(self._index.series_refs_ordered)
def list_timeseries(self, path_prefix: str = "") -> List[str]:
- names = [self._build_series_name(series_ref) for series_ref in
self._index.series_refs_ordered]
if not path_prefix:
- return names
- prefix = path_prefix if path_prefix.endswith(".") else path_prefix +
"."
- return [name for name in names if name.startswith(prefix) or name ==
path_prefix]
+ return [self._build_series_name(series_ref) for series_ref in
self._index.series_refs_ordered]
+
+ try:
+ prefix_parts = split_logical_series_path(path_prefix)
+ except ValueError:
+ return []
+
+ matched = []
+ for series_ref in self._index.series_refs_ordered:
+ device_key, table_entry, field_idx =
self._get_series_components(series_ref)
+ components = build_logical_series_components(
+ table_entry.table_name,
Review Comment:
May use device_key[0] for better readability.
It is confusing at first glance that it suddenly to [1]
--
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]