SchemaHandler - Cleanup string handling and fix syntax error

[colemanw]

Overview

Note: This is a small piece of #25871 which was in there to fix a test failure but feels like it belongs in its own PR for easier review.

The SchemaHandler is a utility which takes an array of table properties and outputs a CREATE/ALTER TABLE SQL string. It contained a bug which wrote invalid sql when creating a table with indexes, and while I was in there I cleaned up some function signatures to clarify they they all output strings.

Before

I guess no one had ever tried to make use of this particular functionality before, but it was definitely broken. When creating a table with indexes it would write SQL like:

CREATE TABLE `foo`
...
INDEX_bar ( bar ),
INDEX_baz ( baz );

This causes a fatal error in sql because you can't declare indexes without the INDEX keyword.

After

CREATE TABLE `foo`
...
INDEX INDEX_bar ( bar ),
INDEX INDEX_baz ( baz );

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

This line fixes the bug. The rest is just code cleanup.

[totten]

Code-cleanups look sensible. This usage of buildSearchIndexSQL() looks more consistent with the other usage(s) of buildSearchIndexSQL().

I couldn't find an existing/organic way to r-run. The bug specifically affects creating a new table with new fields+indexes (at the same time). The existing/organic workflows do those as separate steps, which is why we never noticed. (FWIW, they seem fine before+after the patch.)

Sounds like there's implicit coverage by way of 25871.

Merging.

[colemanw]

Thanks @totten - yes there is test cover in #25871

[colemanw]

@totten I just found 2 more oddities in that class: #26282