Skip to content

feat(clickhouse): add premium geo columns to audit schema#131

Merged
lohanidamodar merged 3 commits into
mainfrom
CLO-4575-audit-geo-columns
Jul 13, 2026
Merged

feat(clickhouse): add premium geo columns to audit schema#131
lohanidamodar merged 3 commits into
mainfrom
CLO-4575-audit-geo-columns

Conversation

@lohanidamodar

Copy link
Copy Markdown
Contributor

What

Adds 9 premium geo columns to the ClickHouse audit schema (CLO-4575), so cloud can store the same premium geo attributes on audit logs that it already stores on usage events:

city, continentCode, subdivisions, isp, autonomousSystemNumber, autonomousSystemOrganization, connectionType, connectionUsageType, connectionOrganization

Changes

  • Adapter/ClickHouse.php — 9 optional string attributes in getAttributes() (mirroring country); the 5 lower-cardinality dims (continentCode, subdivisions, connectionType, connectionUsageType, autonomousSystemNumber) added to LOW_CARDINALITY_COLUMNS. Picked up automatically by CREATE TABLE/getColumnDefinition(); no index/sort-key change.
  • Tests extended (schema shape + LowCardinality vs Nullable classification), verified against a live ClickHouse.

Compatibility

Purely additive. Suggested release: 2.7.0 (minor). Consumers pinning ^2.4 auto-resolve.

Notes

The adapter only does CREATE TABLE IF NOT EXISTS (no ADD COLUMN path), so existing deployments need a one-off ALTER … ADD COLUMN. An idempotent ops script is kept out of this PR (untracked, credential-free, run manually).

Add 9 optional geo attributes (city, continentCode, subdivisions, isp,
autonomousSystemNumber, autonomousSystemOrganization, connectionType,
connectionUsageType, connectionOrganization) to the ClickHouse audit
adapter so audit rows can carry premium geo data.

Low-cardinality dimensions (continentCode, subdivisions, connectionType,
connectionUsageType, autonomousSystemNumber) are added to
LOW_CARDINALITY_COLUMNS so they map to LowCardinality(Nullable(String));
the high-cardinality ones stay plain Nullable(String). createTable() and
getColumnDefinition() pick them up automatically from getAttributes();
no index/sort-key changes.

Extend adapter tests to cover the new attributes and their
LowCardinality vs String/Nullable column-type classification.
@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds 9 optional premium geo columns (city, continentCode, subdivisions, isp, autonomousSystemNumber, autonomousSystemOrganization, connectionType, connectionUsageType, connectionOrganization) to the ClickHouse audit schema, mirroring the existing country attribute definition throughout. Because getColumnDefinition(), createBatch(), and getSelectColumns() all derive their column lists dynamically from getAttributes(), no further logic changes were required.

  • Schema change (ClickHouse.php): 9 new required=false string attributes appended to getAttributes(); 4 lower-cardinality dims added to LOW_CARDINALITY_COLUMNS; a comment explains why autonomousSystemNumber is intentionally excluded.
  • Tests (ClickHouseTest.php): three new test methods cover attribute shape, correct LowCardinality vs Nullable(String) mapping per column, and a write/read round-trip against a live ClickHouse instance using a unique actorId.
  • Migration note: the adapter only emits CREATE TABLE IF NOT EXISTS, so existing deployments require a manual ALTER TABLE … ADD COLUMN for each of the 9 new columns; this is acknowledged in the PR description.

Confidence Score: 5/5

Purely additive schema extension with no changes to existing query, insert, or selection logic.

All new columns are optional, default-null strings that slot into the existing dynamic attribute pipeline without touching any conditional logic. The four LowCardinality choices are intentional and commented. Three new tests — attribute shape, column-type classification, and a live round-trip — give good confidence the columns are correctly defined, typed, and persisted.

No files require special attention.

Important Files Changed

Filename Overview
src/Audit/Adapter/ClickHouse.php Adds 9 optional geo columns to getAttributes() and 4 low-cardinality entries to LOW_CARDINALITY_COLUMNS; getColumnDefinition() and createBatch() pick them up automatically with no other logic changes needed.
tests/Audit/Adapter/ClickHouseTest.php Extends the attribute list test and adds three new targeted tests: attribute shape validation, LowCardinality vs Nullable classification, and a full write/read round-trip using a unique actorId.

Reviews (2): Last reviewed commit: "test(clickhouse): geo round-trip test; m..." | Re-trigger Greptile

Comment thread src/Audit/Adapter/ClickHouse.php Outdated
…lumns

# Conflicts:
#	src/Audit/Adapter/ClickHouse.php
#	tests/Audit/Adapter/ClickHouseTest.php
- autonomousSystemNumber: plain Nullable(String) instead of LowCardinality
  (~100k ASNs globally can exceed ClickHouse's LowCardinality sweet spot;
  the remaining low-card geo dims have tight cardinality). Addresses review.
- Add testPremiumGeoRoundTrip: writes a log with all 9 geo fields and reads
  them back unchanged, proving the columns are created/written/selected.
@lohanidamodar lohanidamodar merged commit a613b29 into main Jul 13, 2026
4 checks passed
@lohanidamodar lohanidamodar deleted the CLO-4575-audit-geo-columns branch July 13, 2026 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant