Skip to content

table: fix shared query filtering#6166

Merged
notxvilka merged 2 commits into
rizinorg:devfrom
IndAlok:table-filter-fix
Apr 8, 2026
Merged

table: fix shared query filtering#6166
notxvilka merged 2 commits into
rizinorg:devfrom
IndAlok:table-filter-fix

Conversation

@IndAlok
Copy link
Copy Markdown
Contributor

@IndAlok IndAlok commented Apr 6, 2026

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository.
  • I made sure to follow the project's coding style.
  • I've documented every RZ_API function and struct this PR changes.
  • I've added tests that prove my changes are effective (required for changes to RZ_API).
  • I've updated the Rizin book with the relevant information (if needed).
  • I've used AI tools to generate fully or partially these code changes and I'm sure the changes are not copyrighted by somebody else.

Detailed description

This PR fixes a table filtering issue that was reported while testing table query syntax on binaries. (as described in rizinorg/book#159, i used curl-example-dbg from testbins for reproducibility and testing)

The issue is the shared table query/filter engine behind commands like izz, is and other table cmds. This caused cmds such as:

izz:string/minlen/8:length/sort/rev:*/page/0/15:csv

to emit rz_num_calc errors on ordinary str contents, and cmds such as:

is:name/uniq:addr/gt/0x1000:name/str/init:addr/sort:json

to behave incorrectly when the query used common names like addr or length that do not always exactly match the underlying table headers.

The changes do three things:

  1. They make num parsing lazy in rz_table_query() / table_filter().
    Before this change, row values were eagerly sent to rz_num_math() even for str-only operations such as str, minlen, and maxlen. That is what produced the rz_num_calc errors.
    After this change, num parsing only happens for operators that actually require num semantics.

  2. They make string eq/ineq stay str based on str columns.
    This avoids accidental numeric treatment of values such as "16", "010", and "0x10" when the column itself is txt.

  3. They add query time column resolution for stable aliases such as addr -> vaddr/address/paddr and length -> len, while preserving the existing cols behavior expected by older query chains.

Tests were also added:

  • new unit coverage was added for the shared table query regressions
  • db tests were added for the izz string/minlen case and the is addr-alias case
  • the addr-alias db test was made deterministic by sorting by name after filtering by addr, so it doesn't depend on platform specific ordering of eq-addr rows

Test plan

Before:
before1
before2

After:
after1
after2

Closing issues

closes #5993

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 6, 2026

Codecov Report

❌ Patch coverage is 45.10870% with 101 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.24%. Comparing base (b11bb9d) to head (9325f01).
⚠️ Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
librz/util/table.c 45.10% 61 Missing and 40 partials ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
librz/util/table.c 63.71% <45.10%> (-3.04%) ⬇️

... and 11 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b11bb9d...9325f01. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@wargio wargio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split this into 2 PRs. one for windows console and one for the table query

@IndAlok
Copy link
Copy Markdown
Contributor Author

IndAlok commented Apr 7, 2026

split this into 2 PRs. one for windows console and one for the table query

Detailed description (PR)

This PR fixes two separate problems (however testing one lead to identification of another, hence could be considered ig)

I thought in this case, could be considered. Np, will split.

@wargio
Copy link
Copy Markdown
Member

wargio commented Apr 7, 2026

split this into 2 PRs. one for windows console and one for the table query

Detailed description (PR)

This PR fixes two separate problems (however testing one lead to identification of another, hence could be considered ig)

I thought in this case, could be considered. Np, will split.

i think you should fix first only the table, then we can fix the terminal

@IndAlok IndAlok marked this pull request as draft April 7, 2026 04:49
@IndAlok IndAlok force-pushed the table-filter-fix branch from ba9dfab to 5252255 Compare April 7, 2026 04:53
@IndAlok IndAlok marked this pull request as ready for review April 7, 2026 04:55
@IndAlok IndAlok changed the title table: fix shared query filtering and Windows console VT restore table: fix shared query filtering Apr 7, 2026
Comment thread librz/util/table.c
Comment on lines +129 to +149
static const char *const addr_aliases[] = {
"vaddr",
"address",
"paddr",
NULL
};
static const char *const length_aliases[] = {
"len",
NULL
};
if (table_index_of_column(t, name, index)) {
return true;
}
// Keep stable query names working across tables with slightly different headers.
if (RZ_STR_EQ(name, "addr")) {
for (size_t i = 0; addr_aliases[i]; i++) {
if (table_index_of_column(t, addr_aliases[i], index)) {
return true;
}
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think you should check the type. usually when you define a row you use xxssXsu etc.. to define if is a number or a string.

Copy link
Copy Markdown
Contributor Author

@IndAlok IndAlok Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, now table_column_is_numeric() treats NUMBER and BOOL cols as num comaptible, table_query_index_of_numeric_column() resolves header name first & rejects it unless the target col is num, & in table_query_index_of_column(), the addr and length alias fallback goes through the num only helper...
have used ex2 and 3 from book for test as well

Copy link
Copy Markdown
Member

@wargio wargio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a quite nice improvement.

Please test the book queries: https://book.rizin.re/src/tools/rz-bin/tables.html

@IndAlok
Copy link
Copy Markdown
Contributor Author

IndAlok commented Apr 7, 2026

it's a quite nice improvement.

Please test the book queries: https://book.rizin.re/src/tools/rz-bin/tables.html

ex 2 and 3 seem to be successfull (#6166 (comment)), however, for ex1, this is the quote from book:
Note: Table specifiers are applied from left to right. Output format specifiers must be specified at the end.

and this is the ex1 provided,
aflt:addr/cols/name/nbbs:size/gt/32:nbbs/gt/1:nbbs/lt/10:nbbs/sort/rev:fancy

addr/cols/name/nbbs is handled by table_query_select_columns(), which turns that into a selected col list and then calls rz_table_columns_select(), and table has 3 cols left only: addr, name & nbbs. then size/gt/32 is there, but size no longer exists in the table, because cols already removed it, and hence we get ERROR: table: Invalid column (size).

however, aflt:size/gt/32:nbbs/gt/1:nbbs/lt/10:addr/cols/name/nbbs:nbbs/sort/rev:fancy would work as size & nbbs is still present when it is filtered and sorted, and after that we reduce the cols...

@IndAlok IndAlok requested a review from wargio April 7, 2026 05:51
@IndAlok IndAlok force-pushed the table-filter-fix branch from 569f6a2 to 9325f01 Compare April 7, 2026 22:03
@notxvilka notxvilka merged commit 300af08 into rizinorg:dev Apr 8, 2026
49 of 50 checks passed
@IndAlok IndAlok deleted the table-filter-fix branch April 8, 2026 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Filtering izz tables (possibly other tables as well) throws errors.

3 participants