fix:remove wrong committed docs and add test test_auto_create_output_…#34987
fix:remove wrong committed docs and add test test_auto_create_output_…#34987guanshengliang merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request removes a documentation file regarding stream deployment timing and introduces a new Python test suite, TestStreamAutoCreateOutputTable, to verify the automatic creation of output tables during stream processing. The review feedback identifies critical issues where test assertions were defined but never executed and a logic error in a row count check. Additionally, suggestions were made to improve code readability by refactoring complex lambda functions and resolving class name shadowing to adhere to better Python practices.
test/cases/18-StreamProcessing/99-Others/test_auto_create_output_table.py
Show resolved
Hide resolved
test/cases/18-StreamProcessing/99-Others/test_auto_create_output_table.py
Show resolved
Hide resolved
| tdSql.checkResultsByFunc( | ||
| sql=result_sql, | ||
| func=lambda: tdSql.getRows() == 2 | ||
| and tdSql.compareData(0, 0, "out4_tb1") | ||
| and tdSql.compareData(0, 1, "tb1") | ||
| and tdSql.compareData(0, 2, "1") | ||
| and tdSql.compareData(1, 0, "out4_tb2") | ||
| and tdSql.compareData(1, 1, "tb2") | ||
| and tdSql.compareData(1, 2, "2") | ||
| ) |
There was a problem hiding this comment.
This lambda function is very long and hard to read due to multiple and conditions. Consider refactoring it into a separate helper function for better readability and maintainability. This would make the test easier to understand and debug.
For example:
def _check_out_ctb4_tags():
if tdSql.getRows() != 2:
return False
if not tdSql.compareData(0, 0, "out4_tb1"): return False
if not tdSql.compareData(0, 1, "tb1"): return False
if not tdSql.compareData(0, 2, "1"): return False
if not tdSql.compareData(1, 0, "out4_tb2"): return False
if not tdSql.compareData(1, 1, "tb2"): return False
if not tdSql.compareData(1, 2, "2"): return False
return True
...
tdSql.checkResultsByFunc(
sql=result_sql,
func=_check_out_ctb4_tags
)| class StreamItem: | ||
| def __init__(self, sql, checkfunc): | ||
| self.sql = sql | ||
| self.checkfunc = checkfunc | ||
|
|
||
| def check(self): | ||
| self.checkfunc() No newline at end of file |
There was a problem hiding this comment.
The StreamItem class is defined as a nested class within TestStreamAutoCreateOutputTable. It's generally better practice in Python to define helper classes like this at the module level to improve readability and reusability.
Additionally, you are importing StreamItem from new_test_framework.utils at the top of the file, but then shadowing it with this local definition. This can be very confusing. Please consider either using the imported StreamItem or renaming your local class to avoid the name collision (e.g., _StreamItem).
There was a problem hiding this comment.
Pull request overview
This pull request removes an incorrectly committed internal documentation file and adds a new StreamProcessing test case intended to validate auto-creation behavior for stream output tables/subtables.
Changes:
- Added a new Python test
test_auto_create_output_table.pycovering stream output-table auto-creation scenarios (ctb/ntb). - Removed
docs/stream_create_deploy_sequence.mdfrom the docs tree.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| test/cases/18-StreamProcessing/99-Others/test_auto_create_output_table.py | New test exercising stream output table/subtable auto-create behavior and result validation. |
| docs/stream_create_deploy_sequence.md | Deleted a previously committed documentation note (appears unintended/incorrect for the repo docs set). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result_sql = f"select c1,tag_tbname from out_ctb1 order by ts;" | ||
| tdSql.checkResultsByFunc( | ||
| sql=result_sql, | ||
| func=lambda: tdSql.getRows() == 2 | ||
| and tdSql.compareData(0, 0, "10001") | ||
| and tdSql.compareData(0, 1, "tb1") | ||
| and tdSql.compareData(1, 0, "10002") | ||
| and tdSql.compareData(1, 1, "tb2") | ||
| ) | ||
| result_sql = f"select c1,tag_tbname from out_ctb2 order by ts;" | ||
| tdSql.checkResultsByFunc( | ||
| sql=result_sql, | ||
| func=lambda: tdSql.getRows() == 2 | ||
| and tdSql.compareData(0, 0, "10001") | ||
| and tdSql.compareData(0, 1, "tb1") | ||
| and tdSql.compareData(1, 0, "10002") | ||
| and tdSql.compareData(1, 1, "tb2") | ||
| ) | ||
| result_sql = f"select c1,tbname,nameoftbl from out_ctb3 order by ts;" | ||
| tdSql.checkResultsByFunc( | ||
| sql=result_sql, | ||
| func=lambda: tdSql.getRows() == 2 | ||
| and tdSql.compareData(0, 0, "10001") | ||
| and tdSql.compareData(0, 1, "out3_tb1") | ||
| and tdSql.compareData(0, 2, "tb1") | ||
| and tdSql.compareData(1, 0, "10002") | ||
| and tdSql.compareData(1, 1, "out3_tb2") | ||
| and tdSql.compareData(1, 2, "tb2") | ||
| ) | ||
| result_sql = f"select c1,tbname,nameoftbl,tagt1 from out_ctb4 order by ts;" | ||
| tdSql.checkResultsByFunc( | ||
| sql=result_sql, | ||
| func=lambda: tdSql.getRows() == 2 | ||
| and tdSql.compareData(0, 0, "10001") | ||
| and tdSql.compareData(0, 1, "out4_tb1") | ||
| and tdSql.compareData(0, 2, "tb1") | ||
| and tdSql.compareData(0, 3, "1") | ||
| and tdSql.compareData(1, 0, "10002") | ||
| and tdSql.compareData(1, 1, "out4_tb2") | ||
| and tdSql.compareData(1, 2, "tb2") | ||
| and tdSql.compareData(1, 3, "2") | ||
| ) | ||
| result_sql = f"select * from out_normal order by ts;" | ||
| tdSql.checkResultsByFunc( | ||
| sql=result_sql, | ||
| func=lambda: tdSql.getRows() == 1 | ||
| and tdSql.compareData(0, 0, "2025-01-01 00:00:00") | ||
| and tdSql.compareData(0, 1, "10001") | ||
| ) |
There was a problem hiding this comment.
tdSql.compareData() ultimately does a strict != comparison against the Python value returned by the driver. In this file, several numeric columns are compared to strings (e.g., "10001", "1"), which will fail if the driver returns ints (as used in other stream tests). Use integers for integer columns (and ensure timestamp formatting matches what the driver returns) to avoid type-mismatch failures.
| and tdSql.compareData(0, 2, "1") | ||
| and tdSql.compareData(1, 0, "out4_tb2") | ||
| and tdSql.compareData(1, 1, "tb2") | ||
| and tdSql.compareData(1, 2, "2") |
There was a problem hiding this comment.
In checks4, tagt1 is defined as an int in the stream DDL, but the assertions compare it to string values ("1"/"2"). Because compareData is type-strict, this is likely to fail; compare against integers instead.
| and tdSql.compareData(0, 2, "1") | |
| and tdSql.compareData(1, 0, "out4_tb2") | |
| and tdSql.compareData(1, 1, "tb2") | |
| and tdSql.compareData(1, 2, "2") | |
| and tdSql.compareData(0, 2, 1) | |
| and tdSql.compareData(1, 0, "out4_tb2") | |
| and tdSql.compareData(1, 1, "tb2") | |
| and tdSql.compareData(1, 2, 2) |
| from new_test_framework.utils import tdSql, tdLog, tdStream, StreamItem | ||
| from new_test_framework.utils.eutil import findTaosdLog |
There was a problem hiding this comment.
The imports StreamItem and findTaosdLog are unused in this test, and the file also defines an inner StreamItem class which shadows the imported StreamItem. This makes the intent unclear and can confuse future refactors; either remove the unused imports or use the framework StreamItem instead of redefining it here.
| from new_test_framework.utils import tdSql, tdLog, tdStream, StreamItem | |
| from new_test_framework.utils.eutil import findTaosdLog | |
| from new_test_framework.utils import tdSql, tdLog, tdStream |
test/cases/18-StreamProcessing/99-Others/test_auto_create_output_table.py
Show resolved
Hide resolved
| result_sql = f"select tags nameoftbl from out_ctb3 order by nameoftbl;" | ||
| tdSql.checkResultsByFunc( | ||
| sql=result_sql, | ||
| func=lambda: tdSql.getRows() == 1 |
There was a problem hiding this comment.
In checks3, the predicate requires tdSql.getRows() == 1 but then compares both row 0 and row 1 (compareData(1, ...)). This will always fail (or raise) when the query returns a single row. Update the expected row count and/or comparisons to be consistent.
| func=lambda: tdSql.getRows() == 1 | |
| func=lambda: tdSql.getRows() == 2 |
4c1856d to
af79217
Compare
|
@Pengrongkun 你这个没加到 cases.task 中吧 |
…table.py
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.