* feat: allow widening the time index column's timestamp unit via ALTER TABLE ... MODIFY COLUMN
Previously MODIFY COLUMN rejected the time index column outright. Now the
time index unit can be widened (Second -> Milli -> Micro -> Nano), which is
lossless for data that fits the target unit: historical data in old SSTs is
cast to the new unit on read by the existing schema-compat layer, and
compaction rewrites it lazily. Narrowing and non-timestamp targets remain
rejected; tag columns keep being rejected. Widening is rejected if any
SST's time range would overflow the target unit's i64 range (e.g.
millisecond -> nanosecond beyond year 2262), since the cast would silently
null those values.
Read-path correctness for old-unit SSTs (verified by new engine e2e tests
and sqlness WHERE queries):
- row-group min/max pruning: parquet statistics of a timestamp column are
raw integers in the file's unit; when the region metadata's type differs
(also the case for altered field columns), stats are now interpreted in
the file's type and converted to the expected type before pruning.
Without this, a new-unit predicate silently pruned whole row groups of
old-unit files (wrong results, rows missing).
- SST-level simple filters are skipped for columns whose file type differs
from the expected type; the predicate is applied by the query layer's
residual filter above the region scan. Also drop the stale
"timestamp columns cannot change type" debug_assert.
- retry idempotency: a same-type ModifyColumnType on the time index
validates as a no-op and `need_alter` returns false, so a retried alter
procedure (region already altered before the previous attempt failed)
converges instead of aborting forever.
- add TimeUnit ordering and ConcreteDataType::is_timestamp_unit_widening_to
- relax ModifyColumnType validation in store-api and table metadata
- tests: unit tests in datatypes/store-api/table; mito2 engine e2e tests
(flushed SST + new writes + reopen + retry + overflow + predicate scans,
cross-unit dedup, mixed-unit SSTs, compaction); sqlness cases incl.
partitioned table and WHERE filters over old-unit data
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: resolve parquet filter issue
* test: provide sqlness tests
* refactor: drop trivial test cases and shorten comments
Review pass over the branch's additions:
- datatypes: keep a representative subset of the widening-matrix asserts
- table: collapse the three single-branch rejection blocks into one loop
- mito2: drop the boundary gt_eq and post-compaction predicate asserts
(covered by the exact-filter regression test and sqlness); drop the
engine-level gt_eq/lt_eq casts (full operator matrix stays in the
cast_timestamp_unit unit tests)
- sqlness: drop a bare full scan already covered by the filter above it
- shorten function doc comments across datatypes/store-api/table/mito2/
recordbatch to the essential semantics
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: drop physical prefilter for columns whose file type differs
Follow-up to the review feedback on CompatBatch/prune reader/filter
handling for widened time index units.
Between/InList/IsNull predicates are prefiltered by PhysicalFilterContext,
which builds its physical expression against the FILE's schema while the
predicate literals are in the expected (post-alter) unit. Evaluating them
against an old-unit SST raised a cross-unit comparison error (Timestamp(ms)
>= Timestamp(µs)) that failed the whole scan. Physical prefilter predicates
are best-effort pruning hints (the query layer re-applies them above the
scan), so drop the prefilter when the column's file type differs from the
expected type, mirroring the simple-filter strategy.
Verified: Between and a non-rewritten (large) InList on old-unit data no
longer error and filter exactly end-to-end (sqlness), and no matching rows
are lost at the engine level (engine test).
Signed-off-by: Ning Sun <sunning@greptime.com>
* test: add direct unit tests for stats cast and prefilter drop
The two-step stats cast (reinterpret raw Int64 stats in the file's
timestamp type, then rescale to the expected type) and the physical
prefilter drop on file/expected type mismatch were only covered
end-to-end; add localized unit tests so a regression fails at the
exact site:
- stats.rs (previously no tests): RowGroupPruningStats min/max over a
hand-built RowGroupMetaData — passthrough with no expected metadata,
passthrough on same type, and rescale (1000ms -> 1_000_000us, not
1000us) on a widened expected unit
- reader.rs: PhysicalFilterContext::new_opt keeps a Between prefilter
when file and expected types match and drops it on unit mismatch
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: tolerate mixed time units range cache key coverage check
* docs: flag mixed-unit hazard in the (unwired) series index
The series index stores per-series min/max ts as raw Int64 in the unit
of the region metadata at write time, and the searcher builds its range
predicates from a single per-region metadata. After a time index unit
widen, files of one region would carry mixed units, so a per-file unit
(or an index rebuild on such alters) is required before this index is
wired into scans. Leave notes at both sites.
Signed-off-by: Ning Sun <sunning@greptime.com>
* test: cover mixed-unit compaction for sparse encoding and strict windows
Compaction-path audit follow-up. The compat cast and window math were
already covered for dense regions; add the two remaining e2e scenarios:
- sparse primary key encoding (used by metric-engine physical regions):
widening then compacting mixed-unit files rewrites the old-unit time
index correctly through the sparse compaction compat path
- strict-window manual compaction: each window output trims rows with a
predicate built in the region's new unit against an old-unit file;
every instant must survive exactly once (no loss, no cross-window
duplication), rescaled
Also documents the audit finding that Regular ranged (manual)
compaction never trims rows: TwcsPicker sets output_time_range to None
and the request time range only selects candidate windows.
Signed-off-by: Ning Sun <sunning@greptime.com>
* test: cover time index unit change in FlatCompatBatch directly
The compat layer's rescaling of a widened time index was only verified
end-to-end; add direct unit tests for both paths:
- dense: identical units skip compat entirely; a widened unit rescales
the time index column (1000ms -> 1_000_000us, not reinterpreted) while
other columns pass through and the output schema matches the expected
metadata
- compact sparse (the metric-engine compaction path): same rescaling
Signed-off-by: Ning Sun <sunning@greptime.com>
* refactor: move timestamp unit division into common-time
The exact unit division (UnitQuotient + div_mod_units) is time
semantics, not filter logic; move it next to TimeUnit in common-time
with a compact test covering representable/non-representable values,
negative (floor) instants, and quotient overflow. The ScalarValue
helpers stay in filter.rs since common-time has no datafusion
dependency.
Signed-off-by: Ning Sun <sunning@greptime.com>
* test: compat rescales a widened time index and fills an added column together
The realistic multi-alter sequence (widen at T1, add column at T2,
read a T0 SST) exercises cast and default-fill in the same
compute_index_and_fields pass; assert both in one output batch.
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: preflight time index widening overflow before any region alters
Address review feedback on the overflow guard:
- preflight: when the frontend operator receives a widening alter on the
time index, run an existence scan (ts outside the target unit's i64
range, LIMIT 1, via the query engine so it covers every region of the
table in both standalone and distributed modes) BEFORE any DDL task is
submitted. A region that fits can no longer commit the new schema
while another region rejects the alter with a non-retryable error.
File and row-group pruning keep the scan cheap when nothing overflows.
The per-region check in mito2 stays as the final guard for data
written after the preflight (the remaining race window); without a
validate-only wire field (region.proto lives in the external
greptime-proto repo) a fully atomic two-phase validate/commit is out
of scope here.
- fast path: cast_timestamp_unit returns the filter unchanged when the
literal is already in the target unit, skipping the div-mod rebuild.
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: address review comments
* fix: address auto review comments
* fix: remove time index widening overflow preflight
Overflow needs timestamps beyond the target unit's i64 range (~year
2262 for nanoseconds), which real workloads never write, so the two
existence scans before every widening alter are not worth the cost.
Region validation already rejects the alter when an SST's time range
overflows the target unit; it now logs the rejection (with the
offending file) and returns a deterministic client-facing message.
Signed-off-by: Ning Sun <sunning@greptime.com>
* test: make sqlness test stable
* fix: log instead of rejecting time index widening overflow
Overflowing values cast to NULL on read but do not otherwise affect
reads or writes, so the alter is allowed; the region-level check now
only logs (with the offending file) when an SST's time range exceeds
the target unit's i64 range.
Signed-off-by: Ning Sun <sunning@greptime.com>
---------
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: postgres describe for more statements
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: cover more show statements
Signed-off-by: Ning Sun <sunning@greptime.com>
* fix: address review comments
- add missing `clippy::too_many_arguments` allow on
`query_from_information_schema_dataframe` (CI clippy failure)
- take `&ShowKind` in the information-schema dataframe helper so `kind`
is no longer cloned at every call site; only the WHERE arm (which needs
an owned expression for `sql_to_expr`) clones internally
- document why re-applying TQL explain formats never overwrites an
existing value (per-query context state)
Signed-off-by: Ning Sun <sunning@greptime.com>
* chore: trim comments to essentials
Signed-off-by: Ning Sun <sunning@greptime.com>
---------
Signed-off-by: Ning Sun <sunning@greptime.com>
- Performance: add collect_lightweight_query_load_metrics to walk the
physical plan and read raw metric values without invoking MetricCollector's
plan-node formatting on the normal query hot path before EOF.
- Refactor: extract collect_full_metrics and keep full aggregation/formatting
for verbose analyze output and terminal metrics.
- Test: cover lightweight partial metrics and drop-time query stats reuse.
Files: src/common/recordbatch/src/adapter.rs
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* feat: report region read load in heartbeat
Signed-off-by: WenyXu <wenymedia@gmail.com>
* feat: expose region query stats in information schema
Signed-off-by: WenyXu <wenymedia@gmail.com>
* chore: update sqlness result
Signed-off-by: WenyXu <wenymedia@gmail.com>
* fix: record region query stats on stream drop
Signed-off-by: WenyXu <wenymedia@gmail.com>
* fix: keep region query cpu stats in nanoseconds
Signed-off-by: WenyXu <wenymedia@gmail.com>
---------
Signed-off-by: WenyXu <wenymedia@gmail.com>
* fix(metric-engine): report query load under physical region id
Propagate the physical region ID through the scanner, record batch
stream, and query engine so that query-load metrics (CPU time, scanned
bytes) are attributed to the correct physical region rather than always
to the logical region.
- `src/store-api/src/region_engine.rs` — add `query_load_region_id` to
`ScannerProperties` and `set_query_load_region_id` to `RegionScanner`
trait
- `src/mito2/src/read/seq_scan.rs`,
`src/mito2/src/read/series_scan.rs`,
`src/mito2/src/read/unordered_scan.rs` — implement the new trait
method on each scanner
- `src/common/recordbatch/src/adapter.rs` — carry region id through
`RecordBatchStreamAdapter` into `RecordBatchMetrics`
- `src/table/src/table/scan.rs` — expose region id on `RegionScanExec`
- `src/query/src/datafusion.rs` — extract region id from the physical
plan and set it on the output stream
- `src/query/src/dist_plan/merge_scan.rs` — use metrics-contained region
id in query-load reporting, falling back to the logical region id
- `src/metric-engine/src/engine/read.rs` — set region id on the metric
engine scanner
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* fix(query): satisfy clippy for query load region id
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
* fix(query): ignore missing query load region ids
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
---------
Signed-off-by: Lei, HUANG <ratuthomm@gmail.com>
Co-authored-by: Lei, HUANG <ratuthomm@gmail.com>
* Initial plan
* fix: guard structured json alignment to fix Clippy CI failure
- Add `is_structured_json_field` function that only returns true for
fields with both JSON extension type AND Struct Arrow data type
- Replace all usages of `is_json_extension_type` / `has_json_extension_field`
with `is_structured_json_field` to prevent legacy JSONB binary columns
from entering structured JSON alignment paths
- Fix logic in `FlatProjectionMapper::new_with_read_columns` to guard
JSON type hint concretization for JSON2 columns only
- Fix `create_column` in show_create_table.rs to only emit JSON structure
settings for JSON2 columns
- Move `mod tests` to end of flat_projection.rs to fix clippy::items_after_test_module
- Add tests for legacy JSON behavior
* fix: guard narrow_read_columns_by_json_type_hint with is_structured_json_field check
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* feat: track scan output bytes and use them for read costing
Track the actual byte count of each record batch produced by
`RegionScanExec` and use it in place of the aggregated per-plan
`memory_usage` as the `table_scan` cost input. This avoids double
counting bytes that flow through multiple operators.
A named constant `REGION_SCAN_EXEC_NAME` exposes the plan node name
so downstream metric parsers remain correct if the struct is renamed.
Affected files:
- `src/table/src/table/metrics.rs` -- add `output_bytes` counter
- `src/table/src/table/scan.rs` -- record bytes, export name constant
- `src/query/src/dist_plan/merge_scan.rs` -- consume scan bytes
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* refactor: remove misleading `mem_used` gauge from scan metrics
The `mem_used` gauge was used with `add()` on every output batch, but a
RegionScan does not hold memory across polls — each batch is yielded
immediately to the upstream operator. The gauge semantics were
incorrect (cumulative `add()` on a gauge) and the value duplicated
`output_bytes` anyway.
Affected files:
- `src/table/src/table/metrics.rs` — drop `mem_used` field and methods
- `src/table/src/table/scan.rs` — remove `record_mem_usage` call
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* fix: use stable plan names for scan byte metrics
Expose `plan_name` alongside rendered plan text so scan byte extraction no longer depends on EXPLAIN formatting. Keep old serialized metrics compatible by defaulting missing `plan_name`.
Affected files:
- `src/common/recordbatch/src/adapter.rs` -- add `plan_name` to `PlanMetrics` and populate it from `ExecutionPlan::name`
- `src/query/src/dist_plan/merge_scan.rs` -- match `REGION_SCAN_EXEC_NAME` through `plan_name`
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* fix: preserve merge scan CPU read cost
Record read cost whenever stream metrics are available, default scan bytes to zero, and aggregate scan output bytes across region scan nodes.
Files: `src/query/src/dist_plan/merge_scan.rs`
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* fix: correct doc comment in `StreamMetrics`
Fix doc comment on `StreamMetrics::new()` to reference the correct
struct name (`StreamMetrics`) instead of the old `MemoryUsageMetrics`.
- `src/table/src/table/metrics.rs` — fix struct name in doc comment
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
---------
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat: struct value
Signed-off-by: Ning Sun <sunning@greptime.com>
* feat: update for proto module
* feat: wip struct type
* feat: implement more vector operations
* feat: make datatype and api
* feat: reoslve some compilation issues
* feat: resolve all compilation issues
* chore: format update
* test: resolve tests
* test: test and refactor value-to-pb
* feat: add more tests and fix for value types
* chore: remove dbg
* feat: test and fix iterator
* fix: resolve struct_type issue
* feat: pgwire 0.33 update
* refactor: use vec for struct items
* feat: conversion from json to value
* feat: add decode function
* fix: lint issue
* feat: update how we encode raw data
* feat: add convertion to fully strcutured StructValue
* refactor: take owned value in all encode/decode functions
* feat: add pg serialization of structvalue
* chore: toml format
* refactor: adopt new and try_new from struct value
* chore: cleanup residual issues
* docs: docs up
* fix lint issue
* Apply suggestion from @MichaelScofield
Co-authored-by: LFC <990479+MichaelScofield@users.noreply.github.com>
* Apply suggestion from @MichaelScofield
Co-authored-by: LFC <990479+MichaelScofield@users.noreply.github.com>
* Apply suggestion from @MichaelScofield
Co-authored-by: LFC <990479+MichaelScofield@users.noreply.github.com>
* Apply suggestion from @MichaelScofield
Co-authored-by: LFC <990479+MichaelScofield@users.noreply.github.com>
* chore: address review comment especially collection capacity
* refactor: remove unneeded processed keys collection
* feat: Value::Json type
* chore: add some work in progress changes
* feat: adopt new json type
* refactor: limit scope json conversion functions
* fix: self review update
* test: provide tests for value::json
* test: add tests for api/helper
* switch proto to main branch
* fix: implement is_null for ValueRef::Json
---------
Signed-off-by: Ning Sun <sunning@greptime.com>
Co-authored-by: LFC <990479+MichaelScofield@users.noreply.github.com>
* feat/kill-process:
### Add Cancellation Support and Enhance Process Management
- **Cancellation Handle Implementation**: Introduced `CancellationHandle` in `cancellation_handle.rs` to facilitate cancellation of futures and streams.
- **Process Management Enhancements**:
- Updated `ProcessManager` in `process_manager.rs` to support cancellable processes using `CancellableProcess`.
- Added `kill_process` method for terminating processes.
- **Stream Wrapper Update**:
- Replaced `StreamWrapper` with `CancellableStreamWrapper` in `stream_wrapper.rs` and `instance.rs` to handle stream cancellation.
- **Error Handling**:
- Added `StreamCancelled` error variant in `error.rs` to handle stream cancellation scenarios.
- **gRPC Handler Update**:
- Added `kill_process` gRPC method in `frontend_grpc_handler.rs` to allow external process termination.
- **Dependency Updates**:
- Updated `Cargo.lock` and `Cargo.toml` to include `common-base` and `tokio-util`.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
**Enhancements and Bug Fixes**
- **Dependency Update**: Updated `greptime-proto` dependency in `Cargo.lock` and `Cargo.toml` to a new revision.
- **Error Handling Improvements**:
- Modified error variants in `src/catalog/src/error.rs` and `src/common/frontend/src/error.rs` to improve error messages and handling.
- Added `FrontendNotFound` error variant for better error specificity.
- **Process Management Enhancements**:
- Updated `ProcessManager` in `src/catalog/src/process_manager.rs` to include `kill_process` functionality with server address validation.
- Enhanced `FrontendClient` trait in `src/common/frontend/src/selector.rs` to support `kill_process` requests.
- **gRPC Handler Update**:
- Refactored `FrontendGrpcHandler` in `src/servers/src/grpc/frontend_grpc_handler.rs` to handle `kill_process` requests asynchronously and return process status.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
### Add Kill Process Functionality
- **`Cargo.lock`, `Cargo.toml`**: Added `common-frontend` as a dependency.
- **`server.rs`, `builder.rs`, `instance.rs`**: Updated `FrontendInvoker` and `FrontendBuilder` to support process management.
- **`error.rs`**: Introduced `InvalidProcessId` error for handling invalid process IDs.
- **`statement.rs`, `kill.rs`**: Implemented `execute_kill` method in `StatementExecutor` to handle the `KILL` statement.
- **`parser.rs`, `statement.rs`**: Updated SQL parser to recognize and parse the `KILL` statement.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
## Add Cancellation Support to Query Execution
- **`process_manager.rs`**: Updated `CancellationHandle` initialization to use `default()` method.
- **`cancellation_handle.rs`**: Implemented `Debug` trait for `CancellationHandle` and added `Cancellation` and `CancellableFuture` structs to support cancellable futures.
- **`error.rs`**: Introduced `Cancelled` error variant to handle query cancellations.
- **`instance.rs`**: Integrated `CancellableFuture` to manage query execution with cancellation support.
- **`stream_wrapper.rs`**: Modified `CancellableStreamWrapper` to use the new `waker()` method for cancellation handling.
- **`statement.rs`**: Added `#[allow(clippy::too_many_arguments)]` to `StatementExecutor::new` to suppress clippy warnings.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
- **Add `MetaClientMissing` Error**: Introduced a new error variant `MetaClientMissing` in `error.rs` to handle missing meta client scenarios.
- **Refactor Cancellation Handling**: Merged `cancellation_handle.rs` into `cancellation.rs` and updated related logic in `process_manager.rs`, `instance.rs`, and `stream_wrapper.rs`.
- **Enhance Process Management**: Improved process management logic in `process_manager.rs` to handle process cancellation more effectively.
- **Update Tests**: Added and updated tests in `cancellation.rs` and `stream_wrapper.rs` to cover new cancellation logic and error handling.
- **Cargo.toml Update**: Adjusted workspace settings in `Cargo.toml` for `common-frontend`.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
- **Add Tests for Process Management**: Introduced multiple async tests in `process_manager.rs` to verify query registration, deregistration, cancellation, and process killing functionalities.
- **Update Error Message in SQL Parser**: Modified the expected error message in `parser.rs` to clarify the expected token as a "process id string literal".
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
### Add Process Count Metrics to Catalog
- **`metrics.rs`**: Introduced a new metric `PROCESS_LIST_COUNT` to track the count of running processes per catalog using `IntGaugeVec`.
- **`process_manager.rs`**: Updated `CancellableProcess` to increment and decrement `PROCESS_LIST_COUNT` upon creation and destruction, respectively. Added a `Drop` implementation for `CancellableProcess` to handle metric updates.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
### Fix process removal logic in `process_manager.rs`
- Corrected the condition for removing an entry from the catalog in `ProcessManager` by using `o.get()` instead of `o.get_mut()`.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
- **Error Handling Improvements**:
- Updated status codes for `Error::FrontendNotFound` and `Error::MetaClientMissing` to `StatusCode::Unexpected` in `src/catalog/src/error.rs`.
- Changed `InvokeFrontend` error display message and status code in `src/common/frontend/src/error.rs`.
- Added `ProcessManagerMissing` error in `src/operator/src/error.rs` and updated its handling in `src/operator/src/statement/kill.rs`.
- **Process Management Enhancements**:
- Added documentation for `ProcessManager` and `register_query` in `src/catalog/src/process_manager.rs`.
- Modified `kill_process` response handling in `src/servers/src/grpc/frontend_grpc_handler.rs`.
- **Cancellation Logic Update**:
- Improved cancellation logic in `src/common/base/src/cancellation.rs` to use `compare_exchange` for atomic operations.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
### Add Process Kill Count Metric and Refactor Cancellation Handle
- **Metrics Update**: Added a new metric `PROCESS_KILL_COUNT` in `metrics.rs` to track the count of completed kill process requests per catalog.
- **Refactor Cancellation Handle**: Renamed `cancellation_handler` to `cancellation_handle` across multiple files for consistency:
- `process_manager.rs`
- `instance.rs`
- `stream_wrapper.rs`
- **Process Management**: Updated process management logic in `process_manager.rs` to increment the `PROCESS_KILL_COUNT` metric upon successful process termination.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
Update metric description in `metrics.rs`
- Changed the description of `PROCESS_KILL_COUNT` to reflect the count of killed processes instead of running processes in `metrics.rs`.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat/kill-process:
Update `greptime-proto` Dependency and Fix Response Field
- **Updated Dependency**: Changed the `greptime-proto` Git revision in `Cargo.lock` and `Cargo.toml` to `f0913f1`.
- **Code Fix**: Modified `frontend_grpc_handler.rs` to correct the response field from `found` to `success` in `KillProcessResponse`.
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
---------
Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
* feat: cache regex in evaluator
* chore: fix warnings
* chore: add reference
* refactor: address CR comments
* Add negative to state
* Don't create the evaluator if the regex is invalid
* test: add test for maybe_build_regex