# Internet SQL 1.25

A correctness release. It fixes `GROUP BY`, which was broken in four separate
ways in 1.24 and in every release before it. No on-disk format or C API has
changed; a 1.22, 1.23 or 1.24 database opens unchanged.

**If you use `GROUP BY`, upgrade and re-run anything that depended on it.**
One of these faults produced wrong numbers rather than an error, so reports
built on grouped totals in any earlier release should be treated as suspect.

---

## Why 1.25 and not another 1.24 build

1.24 was tagged and tested. These binaries answer the same questions
differently, so they cannot honestly report the same version: a bug report
naming "1.24" has to identify one engine, not two.

---

## Fixed — GROUP BY

### Grouped aggregates returned the wrong number

The one to read first, because it did not look like a failure.

Every group summed **the last row of the whole table, once per member row**.
With four salaries 1, 2, 3 and 777 grouped by department:

| | 1.24 | 1.25 |
|---|---|---|
| dept 10 (one row: 1) | `777` | `1` |
| dept 20 (two rows: 2, 3) | `1554` | `5` |
| dept 99 (one row: 777) | `777` | `777` |

`RemapEvalItems` repointed the aggregate's field reference at the per-group
record, but the evaluator reads it through a different member, and that one is
only refreshed inside the type-checking branch — which grouped items, evaluated
without type checking, never reach. It stayed on the source table's field
buffer, which stops moving when the scan ends. Both are now repointed.

### Grouped aggregate columns displayed blank

Two faults on top of the one above. Grouped queries were served out of the row
buffer filled during the table scan — raw rows, in the raw column layout —
while the row count had been changed to the number of groups. The first column
happened to line up, which is why the grouping column looked right and
everything after it looked empty. And the fetch path matches columns by name,
while an aggregate is carried internally as `EXPRESSION` and named in the
result by the user's alias, so the lookup missed. Grouped queries now read the
group output table, and the name match falls back to the alias.

### `GROUP BY` with `ORDER BY` returned rows out of order

```
select deptid from people group by deptid order by deptid;
    1.24:  10, 99, 20          1.25:  10, 20, 99
```

Adding `ORDER BY` actually un-sorted a result that was already in order. The
sort key was read from the raw source buffer at flush time, which by then holds
the first row of the *next* group, so each group was filed under its
successor's key. The same pointer also aimed into a table freed before the sort
ran — a use-after-free that usually appeared to work.

### Two aggregates plus `ORDER BY` crashed the process

```
select deptid,count(*) cnt,sum(salary) total from people
group by deptid order by deptid;
```

exited with `0xC0000005` and no result. Two lists are walked in lockstep during
the ordered fetch, and only the destination was tested for its end; the source
is shorter, because the group output drops the columns that exist only to feed
an aggregate. `count(*)` survived because it adds no argument column, so for it
the two lists were the same length. Guarded, along with three sibling walks
with the same shape.

### A phantom column — and, over a network, garbled values

A field used only inside an aggregate was reported to clients as an extra
column:

```
select deptid,sum(salary) total from people group by deptid;
    1.24:  DEPTID | total | SALARY        1.25:  DEPTID | total
```

Embedded, it was merely an extra blank column. **Over a network connection it
was worse than cosmetic.** The server sends its field definitions before the
query runs, so the definitions and the row bytes described different column
sets, and with three aggregates the client mis-read the row outright:

```
select region,min(amt) mn,max(amt) mx,avg(amt) av from sales group by region;
    embedded, 1.24:  east | 40.00 | 40.00 | 40.00
    remote,   1.24:  east | 0000000     40.00 |  | 0000000    40.00 | 40.00
    both,     1.25:  east | 40.00 | 40.00 | 40.00
```

Anyone reading grouped aggregates through `isqlserverd` in an earlier release
should treat those results as unreliable, not merely untidy.

---

## Diagnostics

Setting `ISQL_DIAG_GROUPBY=1` makes the engine write `isql_groupby_diag.txt`
into the database directory, dumping the record lists the grouping code walks
in lockstep and their contents at each group flush. It is what located the
faults above after static reading produced several confident wrong answers, and
it is left in for the next time.

---

## Verification

- `tests\regress_groupby.ps1` — 7 cases, all passing; committed failing first,
  so each one is known to detect the defect it names.
- `tests\regress_groupby_remote.ps1` — 6 cases run against a live
  `isqlserverd` and compared against the embedded answers, so the two paths
  cannot drift apart again. Checked against the pre-fix binaries: 3 of the 6
  fail there, which is what makes it a test rather than a formality.
- SDK self-test — 29 passed, 0 failed.
- Joins, `DISTINCT`, `WHERE`, plain `ORDER BY` and ungrouped aggregates
  re-checked and unchanged.
- Both drops rebuilt and re-tested after unpacking, not from the build tree.

---

## Unchanged from 1.24

Everything in `RELEASE_NOTES_1.24.md` still applies: the shared command line
across all seven programs, `-cmd`, `-config`, the shell startup repairs, and
the `-p` = password change. That last one still bites 1.23 scripts.

## Known limitations

- **A `storage` column cannot be selected directly.** `select id,vec from …`
  returns `EvalPostLink: Invalid data type in the expression`. A storage column
  is reachable through `VECTOR` and nothing else. Whether that projection
  should be supported is open; the reference documents what the engine does.
- **VLR record format is not implemented.** The syntax is rejected by the
  parser; the VLR documents are a design specification, not a description of
  this build.
- **The PHP interface does not exist as a binary.** `php/php_isql.c` targets
  the PHP 4 Zend API and has never been built.
- **`INSERT` returns the row it inserted**, so it contributes a line to
  `isqlsh -q` output exactly as a `SELECT` does.
