Let the post type wizard index a custom field

Adds an "Index this field" checkbox to each custom field row, off by default.
Ticking it records x-tcms-indexed in the type's sidecar, and saving the type
asks the data model to act on it: on SQLite that is a virtual generated column
plus an index, so a query on that field stops reading every post.  On a model
with no notion of an index it does nothing, which is why the wizard asks the
model rather than checking which one is configured.

A real checkbox, despite the standing warning in post_wizard.js that this is why
'required' is a <select>: the server zips the param_* arrays together
positionally, and an unchecked checkbox submits nothing, which would shift every
later row and land the flag on the wrong field.  The checkbox here carries no
name and never submits; a hidden input beside it does, exactly once per row
either way, and syncIndexedRow keeps the two in step.  The lifecycle test pins
that down with a three field type whose middle box is clear.

The reconciliation is against every post type rather than the one being saved,
because two types can declare a field of the same name and otherwise saving one
would drop the other's index.  Only indexes this created are ever dropped, hence
the prefix; the columns stay, since they cost nothing and re-ticking then only
has to rebuild the index.  Field names are re-validated in the data model rather
than trusted from the wizard -- they reach SQLite as an identifier and a JSON
path, neither of which can be bound as a parameter -- and names the schema owns
are refused outright, which is what stops "index this field" indexing the blob.

Failing to index does not fail the save: the type is written by then, works, and
is only missing an optimisation, so it reports that rather than pretending the
type was not created.

Two bugs found writing the tests.  PRAGMA table_info omits generated columns,
which is every column on posts bar the blob, so the check for "does this column
already exist" saw an empty slate and ADD COLUMN failed the second time a field
was indexed; table_xinfo lists them.  And indexed_fields_for() returned
grep over keys(), whose order Perl randomises per process -- harmless for
private_fields_for(), which is only ever asked whether a field is in it, but this
list is collected across every type and handed to a model to act on.

Also give the sidecar cache sub-second resolution while in here.  It keys on the
mtime of the forms directory at core stat's one second granularity, so a type
written and then read back inside the same second could be answered out of the
cache filled before the write -- a post saved in that window validating against
the schema its type had a moment ago.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6Q1gRJ2Rr85S8FQ47EjnQ

Readme.md

@@ -108,6 +108,16 @@ schema/sqlite.schema.  No migration, because the data itself never moves:
         GENERATED ALWAYS AS (json_extract(post_data, '$.subhead')) VIRTUAL;
     CREATE INDEX posts_subhead ON posts(subhead);
 
+For a post type's own custom fields you don't have to write that yourself: the
+Post Type Wizard offers an "Index this field" checkbox per field, off by default,
+and saving the type does the above for every field that has it ticked.  Untick it
+and the index goes away again on the next save.  It is worth ticking for a field
+you actually search or filter on, and not otherwise -- an index costs a little
+on every save and some disk.
+
+On a data model with no notion of an index the checkbox simply does nothing, so
+the wizard doesn't have to know which model the site is running.
+
 To move an existing flat file site over, run bin/migrate-to-sqlite.pl from the
 tCMS root and then set data_model=SQLite.  It copies rather than moves, so
 data/files is left alone and the way back is to set data_model back.

lib/Trog/Data/SQLite.pm

@@ -359,6 +359,100 @@ sub count ($self) {
     return $count // 0;
 }
 
+=head2 index_fields(@fields)
+
+Give each named custom field a virtual column and an index, and take the indexes
+back off the fields no longer named.
+
+This is what the Post Type Wizard's per-field "Index this field" checkbox does
+when the site is on this data model.  A virtual generated column costs no
+storage and ADD COLUMN on one rewrites nothing, so the expensive half is
+building the index, which is a one-off.
+
+Called with every indexed field across every post type, not just the type that
+was saved -- two types can declare the same field name, and reconciling against
+the whole picture is what stops saving one of them dropping the other's index.
+
+Only indexes this created are ever dropped, which is why they carry a prefix.
+The columns are left in place: they cost nothing, something may reference one,
+and re-ticking the box then only has to rebuild the index.
+
+Field names are re-validated here rather than trusted from the caller -- they
+reach SQLite as an identifier and a JSON path, neither of which can be bound as
+a parameter.
+
+=cut
+
+our $index_prefix = 'posts_custom_';
+
+# The columns schema/sqlite.schema defines for itself.  The wizard already
+# refuses a custom field named after anything in the post schema, which covers
+# most of these; this is the backstop for the ones that are not post fields at
+# all, and it is what stops "index this field" quietly indexing the blob.
+our %reserved = map { $_ => 1 } qw{id post_data uuid version created title user author form aclname visibility local_href data};
+
+sub index_fields ( $self, @fields ) {
+    my $dbh = _dbh();
+
+    my %want;
+    foreach my $field (@fields) {
+        my ($safe) = ( $field // '' ) =~ m/^([a-z][a-z0-9_]{0,31})$/;
+        if ( !defined $safe ) {
+            WARN( "Refusing to index '" . ( $field // '' ) . "': not a field name" );
+            next;
+        }
+        if ( $reserved{$safe} ) {
+            WARN("Refusing to index '$safe': the schema owns that column, and already indexes the ones worth indexing");
+            next;
+        }
+        $want{$safe} = 1;
+    }
+
+    # Whatever is already there, base columns included.  A custom field can
+    # never be named after one of those -- the wizard refuses any name already
+    # in the post schema -- so anything found here is either ours or was added
+    # by hand, and either way there is nothing to add.
+    #
+    # xinfo rather than info: table_info omits generated columns, which is every
+    # column on this table bar the blob, so it would report an empty slate every
+    # time and ADD COLUMN would fail on the second call for the same field.
+    my %have = map { $_->{name} => 1 } @{ $dbh->selectall_arrayref( 'PRAGMA table_xinfo(posts)', { Slice => {} } ) // [] };
+
+    my $built = 0;
+    foreach my $field ( sort keys %want ) {
+        if ( !$have{$field} ) {
+            ## no critic(ValuesAndExpressions::PreventSQLInjection) -- $field is the anchored capture above
+            $dbh->do("ALTER TABLE posts ADD COLUMN $field TEXT GENERATED ALWAYS AS (json_extract(post_data, '\$.$field')) VIRTUAL")
+              or confess "Could not add a column for '$field': " . $dbh->errstr;
+        }
+
+        ## no critic(ValuesAndExpressions::PreventSQLInjection) -- as above
+        $dbh->do("CREATE INDEX IF NOT EXISTS $index_prefix$field ON posts($field)")
+          or confess "Could not index '$field': " . $dbh->errstr;
+        $built++;
+    }
+
+    # Anything we built before and were not asked for this time.
+    my $ours = $dbh->selectall_arrayref(
+        "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'posts' AND name LIKE ?",
+        { Slice => {} },
+        "$index_prefix%"
+    ) // [];
+
+    my $dropped = 0;
+    foreach my $index ( map { $_->{name} } @$ours ) {
+        my $field = substr( $index, length($index_prefix) );
+        next if $want{$field};
+
+        ## no critic(ValuesAndExpressions::PreventSQLInjection) -- $index came out of sqlite_master
+        $dbh->do("DROP INDEX IF EXISTS $index") or confess "Could not drop the index on '$field': " . $dbh->errstr;
+        $dropped++;
+    }
+
+    INFO("Indexed $built custom field(s), dropped $dropped") if $built || $dropped;
+    return $built;
+}
+
 =head2 tags() = @tags
 
 Every tag in use, from the index the schema's triggers maintain.

lib/Trog/DataModule.pm

@@ -14,6 +14,7 @@ use File::Slurper();
 use JSON::MaybeXS();
 use JSON::Validator::Schema::Troglodyne();
 
+use Time::HiRes();
 use Trog::Themes();
 
 use Trog::Log qw{:all};
@@ -394,8 +395,16 @@ picks the change up on its very next add() with no restart needed.
 # Stat the forms dirs, not their parents -- writing blog.json bumps the mtime
 # of forms/, and nothing above it.  Both candidates, since either one gaining a
 # sidecar changes the answer.
+#
+# Time::HiRes::stat, because core stat truncates mtime to the second: the post
+# type wizard writes a sidecar and something reads it back moments later, and at
+# one second of resolution those two are frequently the same generation.  The
+# reader then answers out of the cache it filled before the write, which is how
+# a type saved and used inside the same second validated against the schema it
+# had a moment ago.  Falls back to whatever stat gives where the filesystem
+# carries no sub-second timestamps.
 sub _sidecar_generation {
-    return join( ':', map { ( stat("$_/forms") )[9] // 0 } Trog::Themes::template_dirs( 'text/html', 1 ) );
+    return join( ':', map { ( Time::HiRes::stat("$_/forms") )[9] // 0 } Trog::Themes::template_dirs( 'text/html', 1 ) );
 }
 
 # A signature default only covers an absent arg, and plenty of posts have an
@@ -454,6 +463,43 @@ sub private_fields_for ( $form = '' ) {
     return [ grep { $properties->{$_}{'x-tcms-private'} } keys(%$properties) ];
 }
 
+=head2 indexed_fields_for($form)
+
+The custom fields of a post type which the Post Type Wizard was told to index,
+as declared by x-tcms-indexed in the sidecar.
+
+A field is unindexed unless it says otherwise.  What indexing a field actually
+means is up to the data model -- see index_fields(), which most of them ignore.
+
+=cut
+
+sub indexed_fields_for ( $form = '' ) {
+    my $properties = schema_for($form)->{properties} // {};
+
+    # Sorted, unlike private_fields_for(), which is only ever asked whether a
+    # given field is in it.  This list is collected across every post type and
+    # handed to a data model to act on, and an answer that comes back in a
+    # different order each run makes a reconciliation impossible to reason about.
+    return [ sort grep { $properties->{$_}{'x-tcms-indexed'} } keys(%$properties) ];
+}
+
+=head2 index_fields(@fields)
+
+Tell the data model which custom fields ought to be queryable, and let it do
+whatever that means for it.
+
+The default is to do nothing at all, and that is a perfectly good answer: a
+model with no notion of an index has nothing to build, and the wizard should not
+have to know which model it is talking to in order to offer the option.
+
+Called with the full set every time rather than with a delta, so a model that
+does act on it can bring itself into line in one pass -- including letting go of
+what is no longer asked for.
+
+=cut
+
+sub index_fields ( $self, @fields ) { return 0 }
+
 =head2 relations_for($form)
 
 The x-tcms-relations table for a post type, as a hashref keyed on the template

lib/Trog/Routes/HTML.pm

@@ -2272,6 +2272,11 @@ sub post_wizard_save ($query) {
 
     INFO("Post type '$name' written to $tx_file by $query->{user}");
 
+    # Whatever the data model makes of an indexed field, it wants the whole
+    # picture rather than this type's share of it -- see _wizard_reindex.
+    my $index_error = _wizard_reindex( $name, $fields );
+    return _wizard_fail( $query, "Post type '$name' was written, but indexing its fields failed: $index_error" ) if $index_error;
+
     # Nothing else is needed to publish a new type: themed_templates_in_dir()
     # re-reads
     # the directory every call, and Xslate resolves includes lazily, so both the
@@ -2297,6 +2302,49 @@ sub post_wizard_save ($query) {
     );
 }
 
+=head2 _wizard_reindex($name, $fields)
+
+Bring the data model's idea of which custom fields are queryable into line with
+what the post types on disk declare.
+
+Every type's indexed fields, not just the one that was saved: two types can
+declare a field of the same name, so reconciling against one of them alone would
+have saving that type drop the other's index.
+
+The type being saved is taken from the field list in hand rather than by reading
+back the sidecar just written.  schema_for() caches on the mtime of the forms
+directory, which has one second of resolution, so a read here can legitimately
+land on the generation before this save.
+
+Returns the error if there was one, and nothing if all was well.  A failure to
+index is not a failure to create the post type: the type is written by this
+point, works, and is only missing an optimisation.
+
+=cut
+
+sub _wizard_reindex ( $name, $fields ) {
+    my %indexed = map { $_->{name} => 1 } grep { $_->{indexed} } @$fields;
+
+    my $this_type = "$name.tx";
+    foreach my $form ( @{ Trog::Themes::themed_templates_in_dir( 'forms', 'text/html', 1 ) } ) {
+        next if $form eq $this_type;
+        $indexed{$_} = 1 foreach @{ Trog::DataModule::indexed_fields_for($form) };
+    }
+
+    local $@;
+    eval {
+        my $data = Trog::Data->new( Trog::Config::get() );
+        $data->index_fields( sort keys %indexed );
+        1;
+    } or do {
+        my $error = $@;
+        WARN("Could not reconcile custom field indexes: $error");
+        return $error;
+    };
+
+    return;
+}
+
 # Repeated params arrive as arrayrefs; anywhere we want one string, insist on one string.
 sub _wizard_scalar ( $value = '' ) {
     return '' if !defined $value || ref $value;
@@ -2350,6 +2398,7 @@ sub _wizard_fields ($query) {
     my $rforms  = Trog::Utils::coerce_array( $query->{param_relation_form} );
     my $rmodes  = Trog::Utils::coerce_array( $query->{param_relation_mode} );
     my $private = Trog::Utils::coerce_array( $query->{param_private} );
+    my $indexed = Trog::Utils::coerce_array( $query->{param_indexed} );
 
     my ( @fields, %seen );
     foreach my $index ( 0 .. $#$names ) {
@@ -2383,6 +2432,7 @@ sub _wizard_fields ($query) {
                 relation_form => $rform,
                 relation_mode => $rmode,
                 private       => _wizard_scalar( $private->[$index] ) ? 1 : 0,
+                indexed       => _wizard_scalar( $indexed->[$index] ) ? 1 : 0,
             }
         );
     }
@@ -2424,6 +2474,10 @@ sub _wizard_sidecar ( $name, $fields, $includes, $body_form, $datasource = '' )
         # and a sidecar full of x-tcms-private:false reads like the opposite.
         $property->{'x-tcms-private'} = JSON::MaybeXS::true() if $field->{private};
 
+        # Likewise unindexed unless asked for.  The sidecar is the declaration;
+        # post_wizard_save() reconciles the data model against it.
+        $property->{'x-tcms-indexed'} = JSON::MaybeXS::true() if $field->{indexed};
+
         if ( $field->{type} eq 'relation' ) {
             $property->{'x-tcms-relation-form'} = $field->{relation_form};
 

t/Trog-Data-SQLite.t

@@ -301,4 +301,58 @@ subtest 'writing' => sub {
     unlike( $blob, qr/version_max|"modified"|display_name|user_class/, 'derived fields are not stored' );
 };
 
+subtest 'indexing a custom field' => sub {
+
+    # index_fields() logs what it did, and with no logger configured that comes
+    # out on stderr with a Carp::Always trace behind it.
+    local $SIG{__WARN__} = sub { };
+
+    my $dbh = Trog::SQLite::dbh( undef, 'data/posts.sqlite' );
+
+    # xinfo, because table_info does not list generated columns.
+    my $columns = sub {
+        return { map { $_->{name} => 1 } @{ $dbh->selectall_arrayref( 'PRAGMA table_xinfo(posts)', { Slice => {} } ) } };
+    };
+    my $indexes = sub {
+        return { map { $_->{name} => 1 } @{ $dbh->selectall_arrayref( "SELECT name FROM sqlite_master WHERE type='index' AND tbl_name='posts'", { Slice => {} } ) } };
+    };
+
+    ok( !$columns->()->{cook_time}, 'the field has no column to start with' );
+
+    $sqlite->index_fields(qw{cook_time});
+    ok( $columns->()->{cook_time},                'indexing a field gives it a column' );
+    ok( $indexes->()->{'posts_custom_cook_time'}, 'and an index' );
+
+    # The column is generated out of the blob, so it is populated for posts that
+    # were written long before anybody asked for it.
+    $sqlite->write( [ post( id => 'p-recipe', title => 'Soup', cook_time => '45 minutes' ) ] );
+    my ($value) = $dbh->selectrow_array(q{SELECT cook_time FROM posts WHERE uuid = 'p-recipe'});
+    is( $value, '45 minutes', 'and the column reads the value straight out of the post' );
+
+    # And SQLite actually uses it, which is the entire point.
+    my ($plan) = map { $_->{detail} } @{ $dbh->selectall_arrayref( q{EXPLAIN QUERY PLAN SELECT id FROM posts WHERE cook_time = '45 minutes'}, { Slice => {} } ) };
+    like( $plan, qr/USING INDEX posts_custom_cook_time/, 'and searches it rather than every post' );
+
+    is( exception { $sqlite->index_fields(qw{cook_time}) }, undef, 'asking twice is not an error' );
+
+    # Reconciling against a set that no longer names it.
+    $sqlite->index_fields(qw{serves});
+    ok( !$indexes->()->{'posts_custom_cook_time'}, 'a field left out loses its index' );
+    ok( $columns->()->{cook_time},                 'but keeps its column, which costs nothing' );
+    ok( $indexes->()->{'posts_custom_serves'},     'and the newly named one gains an index' );
+
+    # Only ever ours.
+    $sqlite->index_fields();
+    ok( $indexes->()->{posts_created}, "the schema's own indexes are left alone" );
+    ok( $indexes->()->{posts_title},   'all of them' );
+
+    # The name reaches SQLite as an identifier and a JSON path, neither of which
+    # can be bound as a parameter, so it is checked here and not merely upstream.
+    foreach my $hostile ( 'cook_time; DROP TABLE posts', "cook'time", 'Cook_Time', '1st_course', 'a' x 40, '', 'post_data' ) {
+        is( exception { $sqlite->index_fields($hostile) }, undef, "'$hostile' is refused rather than run" );
+    }
+    ok( $indexes->()->{posts_created},             'and the table is still there afterwards' );
+    ok( !$indexes->()->{'posts_custom_post_data'}, 'and a schema column did not pick up an index of its own' );
+};
+
 done_testing();

t/Trog-Routes-HTML-series-lifecycle.t

@@ -526,6 +526,76 @@ subtest 'a datasource-backed type gets no editor' => sub {
     ok( !-e 'www/templates/html/components/forms/spec_bogus.tx', 'and nothing was written' );
 };
 
+subtest 'a field can be declared indexed' => sub {
+    my ( $code, $body, $err ) = _render(
+        _admin(
+            route          => '/admin/wyzzerdd/save',
+            method         => 'POST',
+            name           => 'spec_indexed',
+            body_form      => 'form_common.tx',
+            wrapper        => 1,
+            inc_post_title => 1,
+
+            # The middle row is the one that matters: its box is clear, and the
+            # wizard zips these arrays together positionally, so if a clear box
+            # ever submitted nothing the flag would land on the wrong field.
+            param_name          => [ 'cook_time', 'garnish', 'serves' ],
+            param_type          => [ 'text',      'text',    'number' ],
+            param_label         => [ 'Cook Time', 'Garnish', 'Serves' ],
+            param_placeholder   => [ '',          '',        '' ],
+            param_required      => [ 0,           0,         0 ],
+            param_private       => [ 0,           0,         0 ],
+            param_indexed       => [ 1,           0,         1 ],
+            param_relation_form => [ 'blog.tx',   'blog.tx', 'blog.tx' ],
+            param_relation_mode => [ 'one',       'one',     'one' ],
+            display             => q{<div class="ct"><: $post.cook_time :></div>},
+        ),
+        \&Trog::Routes::HTML::post_wizard_save,
+    );
+    is( $code, 200, 'the type was created' ) or diag($err);
+
+    my $sidecar = JSON::MaybeXS::decode_json( Path::Tiny->new('www/templates/html/components/forms/spec_indexed.json')->slurp_utf8 );
+    ok( $sidecar->{properties}{cook_time}{'x-tcms-indexed'},         'the ticked field is marked indexed' );
+    ok( $sidecar->{properties}{serves}{'x-tcms-indexed'},            'and so is the one after the clear box' );
+    ok( !exists $sidecar->{properties}{garnish}{'x-tcms-indexed'},   'the field between them is not' );
+    ok( !exists $sidecar->{properties}{cook_time}{'x-tcms-private'}, 'and indexing is not privacy' );
+
+    is_deeply(
+        Trog::DataModule::indexed_fields_for('spec_indexed.tx'),
+        [ sort qw{cook_time serves} ],
+        'and the model reads them back out of the sidecar'
+    );
+
+  SKIP: {
+        skip( "the $MODEL model has nothing to index with", 3 ) unless $MODEL eq 'SQLite';
+
+        my $dbh     = Trog::SQLite::dbh( undef, 'data/posts.sqlite' );
+        my %columns = map { $_->{name} => 1 } @{ $dbh->selectall_arrayref( 'PRAGMA table_xinfo(posts)',                                              { Slice => {} } ) };
+        my %indexes = map { $_->{name} => 1 } @{ $dbh->selectall_arrayref( "SELECT name FROM sqlite_master WHERE type='index' AND tbl_name='posts'", { Slice => {} } ) };
+
+        ok( $columns{cook_time},                'saving the type gave the ticked field a column' );
+        ok( $indexes{'posts_custom_cook_time'}, 'and an index' );
+        ok( !$indexes{'posts_custom_garnish'},  'and left the unticked one alone' );
+    }
+
+    # A post of the type still saves and reads back the same as any other.
+    _reindex();
+    _make_series( 'spec_indexed', 'specindexed', 'spec_indexed.tx', 'Spec Indexed Series', no_topbar => 1 ) or return;
+    my $child = _save_post(
+        'spec_indexed child',
+        form       => 'spec_indexed.tx',
+        title      => 'Spec Indexed Child',
+        cook_time  => '45 minutes',
+        garnish    => 'parsley',
+        visibility => 'public',
+        tags       => ['specindexed'],
+        to         => '/specindexed',
+    ) or return;
+
+    is( $child->{cook_time}, '45 minutes', 'an indexed field stores like any other' );
+    is( $child->{garnish},   'parsley',    'and so does the one beside it' );
+};
+
 subtest 'a field can be declared editors-only' => sub {
     my ( $code, $body, $err ) = _render(
         _admin(

www/scripts/post_wizard.js

@@ -1,9 +1,15 @@
 // Post Type Wizard - repeatable custom field rows.
 //
 // Every row contributes exactly one param_name, param_type, param_label,
-// param_placeholder and param_required, so the five arrive server side as
-// aligned arrays.  This is why 'required' is a <select> and not a checkbox --
-// an unchecked checkbox submits nothing and would shift every later row.
+// param_placeholder, param_required, param_private and param_indexed, so they
+// arrive server side as aligned arrays.  This is why 'required' is a <select>
+// and not a checkbox -- an unchecked checkbox submits nothing and would shift
+// every later row.
+//
+// 'Index this field' is a real checkbox because it reads like one, and keeps
+// the guarantee anyway: the checkbox itself has no name and never submits, and
+// the hidden input beside it -- which always submits, exactly once -- is what
+// syncIndexedRow keeps in step with it.
 
 function addParam() {
     var tpl  = document.getElementById('wizard-param-template');
@@ -42,6 +48,19 @@ function syncRelationRow(select) {
     return false;
 }
 
+// Copy a row's index checkbox into the hidden input that actually submits.
+function syncIndexedRow(checkbox) {
+    var row = checkbox.closest('.wizard-param-row');
+    if (!row) {
+        return false;
+    }
+    var hidden = row.querySelector('input[name="param_indexed"]');
+    if (hidden) {
+        hidden.value = checkbox.checked ? '1' : '0';
+    }
+    return false;
+}
+
 function delParam(button) {
     var row = button.closest('.wizard-param-row');
     if (row) {

www/templates/html/components/post_wizard.tx

@@ -134,6 +134,18 @@ any custom fields, so that the data model knows to keep them when a post is save
             <option value="0">Shown to everyone</option>
             <option value="1">Shown to editors only</option>
         </select>
+        <!--
+            The checkbox has no name, so it never submits; the hidden input beside
+            it carries the value and always submits exactly one, ticked or not.
+            A named checkbox would submit nothing when clear, and the server zips
+            the param_* arrays together positionally, so one short row shifts
+            every row after it.
+        -->
+        <label class="wizard-index" title="Give this field its own column and index, so queries on it do not have to read every post.  Costs a little on save and some disk; only worth it for a field you actually search or filter on.">
+            Index this field
+            <input type="hidden" name="param_indexed" value="0" />
+            <input class="coolcb" type="checkbox" onchange="syncIndexedRow(this);" />
+        </label>
         <button type="button" class="coolbutton" onclick="delParam(this); return false;">Remove</button>
     </div>
 </template>