Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions flow/connectors/mysql/qrep.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,14 @@ func (c *MySqlConnector) GetQRepPartitions(
}
partitionHelper.AddPartitions(uuidPartitions)
} else {
c.logger.Info("string watermark column is not uuid, falling back to full table partition")
return utils.FullTablePartition(), nil
stringPartitions, err := buildAdaptiveStringPartitions(
ctx, c, c.logger, parsedWatermarkTable, config.WatermarkColumn, start, end, numPartitions)
if err != nil {
c.logger.Warn("failed to build adaptive string partitions, falling back to full table partition",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should still be deterministic in face of network errors.
Haven't we had this conversation before
#4097 (comment)
#3972 (comment)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This masks MariaDB rejecting EXPLAIN FORMAT=TRADITIONAL

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

surprised that tests that expect N partitions did not catch this, will take a look what happened here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MariaDB seem to support it just fine:

MariaDB [test]> explain FORMAT=TRADITIONAL SELECT 1 FROM a where id1 > 0 and id1 < 1;
+------+-------------+-------+-------+---------------+---------+---------+------+------+-------------+
| id   | select_type | table | type  | possible_keys | key     | key_len | ref  | rows | Extra       |
+------+-------------+-------+-------+---------------+---------+---------+------+------+-------------+
|    1 | SIMPLE      | a     | range | PRIMARY       | PRIMARY | 4       | NULL | 1    | Using where |
+------+-------------+-------+-------+---------------+---------+---------+------+------+-------------+

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad on Maria, gotta keep my AIs in check

slog.Any("error", err))
return utils.FullTablePartition(), nil
}
partitionHelper.AddPartitions(stringPartitions)
}
} else if err := partitionHelper.AddPartitionsWithRange(val1.Value(), val2.Value(), numPartitions); err != nil {
return nil, fmt.Errorf("failed to add partitions: %w", err)
Expand Down
223 changes: 223 additions & 0 deletions flow/connectors/mysql/qrep_partition.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
package connmysql

import (
"container/heap"
"context"
"fmt"
"log/slog"
"math/big"
"regexp"
"strings"

"github.com/go-mysql-org/go-mysql/mysql"
"github.com/google/uuid"
"go.temporal.io/sdk/log"

"github.com/PeerDB-io/peerdb/flow/connectors/utils"
"github.com/PeerDB-io/peerdb/flow/generated/protos"
"github.com/PeerDB-io/peerdb/flow/pkg/common"
"github.com/PeerDB-io/peerdb/flow/shared"
)

Expand Down Expand Up @@ -100,3 +106,220 @@ func bigIntToUUID(n *big.Int, casing hexCasing) (string, error) {
}
return s, nil
}

func createStringPartition(start string, end string, endInclusive bool) *protos.QRepPartition {
return &protos.QRepPartition{
PartitionId: uuid.NewString(),
Range: &protos.PartitionRange{
Range: &protos.PartitionRange_StringRange{
StringRange: &protos.StringPartitionRange{
Start: start,
End: end,
EndInclusive: endInclusive,
},
},
},
}
}

const (
base95Min = ' ' // 0x20, lowest printable ASCII -> digit 0
base95Max = '~' // 0x7E, highest printable ASCII -> digit 94
base95Radix = base95Max - base95Min + 1
base95Width = 8 // 95^8 to fit in an uint64
)

func stringMidpoint(s1 string, s2 string) string {
i := 0
for i < len(s1) && i < len(s2) && s1[i] == s2[i] {
i++
}
sharedPrefix := s1[:i]
Comment thread
jgao54 marked this conversation as resolved.
s1, s2 = s1[i:], s2[i:]
mid := (stringToBase95Integer(s1) + stringToBase95Integer(s2)) / 2
return strings.TrimRight(sharedPrefix+base95IntegerToString(mid), " ")
}

func stringToBase95Integer(s string) uint64 {
if s == "" {
return 0
}
var res uint64
for i := range base95Width {
var digit uint64
if i < len(s) {
ch := s[i]
switch {
case ch < base95Min:
ch = base95Min
case ch > base95Max:
ch = base95Max
}
digit = uint64(ch - base95Min)
}
res = res*base95Radix + digit
}
return res
}

func base95IntegerToString(n uint64) string {
digits := make([]byte, base95Width)
for k := base95Width - 1; k >= 0; k-- {
digits[k] = base95Min + byte(n%base95Radix)
n /= base95Radix
}
return string(digits)
}

type stringPartitionEntry struct {
start string
end string
rows uint64
}

type stringPartitionHeap []stringPartitionEntry

func (h *stringPartitionHeap) Len() int { return len(*h) }
func (h *stringPartitionHeap) Less(i, j int) bool { return (*h)[i].rows > (*h)[j].rows }
func (h *stringPartitionHeap) Swap(i, j int) { (*h)[i], (*h)[j] = (*h)[j], (*h)[i] }

func (h *stringPartitionHeap) Push(x any) { *h = append(*h, x.(stringPartitionEntry)) }

func (h *stringPartitionHeap) Pop() any {
old := *h
n := len(old)
item := old[n-1]
*h = old[:n-1]
return item
}

// interface for unit-testing
type rangeProber interface {
estimateRowsInRange(ctx context.Context, tableName string, quotedCol string, start string, end string) (uint64, error)
fetchNextRealKey(
ctx context.Context, tableName string, quotedCol string, midpoint string, start string, end string,
) (string, bool, error)
}

// buildAdaptiveStringPartitions splits an arbitrary string watermark column
// into at most numPartitions partitions using midpoint bisection guided
// by the query planner's row estimates. It starts from a single [minVal, maxVal]
// partition and repeatedly splits the largest partition, until it reaches
// numPartitions or runs out of splittable partitions.
func buildAdaptiveStringPartitions(
ctx context.Context,
prober rangeProber,
logger log.Logger,
table *common.QualifiedTable,
watermarkColumn string,
minVal string,
maxVal string,
numPartitions int64,
) ([]*protos.QRepPartition, error) {
tableName := table.MySQL()
quotedCol := common.QuoteMySQLIdentifier(watermarkColumn)

totalRows, err := prober.estimateRowsInRange(ctx, tableName, quotedCol, minVal, maxVal)
if err != nil {
return nil, fmt.Errorf("failed to estimate rows for [%s, %s]: %w", minVal, maxVal, err)
Comment thread
jgao54 marked this conversation as resolved.
}
h := &stringPartitionHeap{{start: minVal, end: maxVal, rows: totalRows}}
heap.Init(h)

var outputs []stringPartitionEntry
for int64(len(outputs)+h.Len()) < numPartitions && h.Len() > 0 {
p := heap.Pop(h).(stringPartitionEntry)

if p.start == p.end {
outputs = append(outputs, p)
continue
}

mid := stringMidpoint(p.start, p.end)
k, found, err := prober.fetchNextRealKey(ctx, tableName, quotedCol, mid, p.start, p.end)
if err != nil {
return nil, fmt.Errorf("failed to fetch next real key for [%s, %s]: %w", p.start, p.end, err)
}
if !found {
outputs = append(outputs, p)
continue
}

leftRows, err := prober.estimateRowsInRange(ctx, tableName, quotedCol, p.start, k)
if err != nil {
return nil, fmt.Errorf("failed to estimate rows for [%s, %s]: %w", p.start, k, err)
}
rightRows, err := prober.estimateRowsInRange(ctx, tableName, quotedCol, k, p.end)
if err != nil {
return nil, fmt.Errorf("failed to estimate rows for [%s, %s]: %w", k, p.end, err)
}
heap.Push(h, stringPartitionEntry{start: p.start, end: k, rows: leftRows})
heap.Push(h, stringPartitionEntry{start: k, end: p.end, rows: rightRows})
}

for h.Len() > 0 {
outputs = append(outputs, heap.Pop(h).(stringPartitionEntry))
}

partitions := make([]*protos.QRepPartition, 0, len(outputs))
for _, p := range outputs {
partitions = append(partitions, createStringPartition(p.start, p.end, p.end == maxVal))
}
logger.Info("[mysql] built adaptive string partitions",
slog.Int64("targetNumPartitions", numPartitions),
slog.Int("numPartitions", len(partitions)))

return partitions, nil
}

// fetchNextRealKey returns the smallest real value of the watermark column that
// is at or past the interpolated midpoint and strictly inside (start, end). The
// bounds are enforced by MySQL using its column's collation. Return false when
// no such key exists.
func (c *MySqlConnector) fetchNextRealKey(
ctx context.Context, tableName string, quotedCol string, midpoint string, start string, end string,
) (string, bool, error) {
query := fmt.Sprintf(
"SELECT %[1]s FROM %[2]s WHERE %[1]s >= '%[3]s' AND %[1]s > '%[4]s' AND %[1]s < '%[5]s' ORDER BY %[1]s LIMIT 1",
quotedCol, tableName, mysql.Escape(midpoint), mysql.Escape(start), mysql.Escape(end))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Critical — Watermark values interpolated with the wrong escaping for the session's sql_mode

flow/connectors/mysql/qrep_partition.go:281 (and :294, :325, plus the pull query at flow/connectors/mysql/qrep.go:379)

Every new probe query and the StringRange pull query build SQL literals with mysql.Escape(...) wrapped in single quotes. But the connector unconditionally runs SET sql_mode = 'ANSI,NO_BACKSLASH_ESCAPES' (mysql.go:237). mysql.Escape does backslash escaping ('\'), which under NO_BACKSLASH_ESCAPES is a no-op — the backslash is a literal character. This was masked at merge-base because non-UUID strings fell back to full-table and emitted no per-value SQL; UUID boundaries are hex-only. This branch routes arbitrary row data through these literals for the first time. Three concrete failure modes, reproduced live on MySQL 8.0:

  • Silent data loss (this is the critical core). A max watermark value like Z:\tmp (Windows/UNC path as key) is stored unescaped, so partitioning succeeds — no fallback. The last partition's inclusive bound becomes BETWEEN ... AND 'Z:\\tmp', which under NO_BACKSLASH_ESCAPES is the literal 7-char string Z:\\tmp. The real Z:\tmp sorts above it, so the max row (and neighbors) are silently never replicated. Verified: SELECT ... BETWEEN 'Mmm' AND 'Z:\\tmp' returned only Mmm, dropping the real max. Symmetric at the min bound with \n/\t/\0. Interior boundaries stay consistent (adjacent partitions share the identical escaped key), so the leak is concentrated at the two endpoints — but it's a successful query with no error and no fallback.
  • SQL injection. A source row value like x' OR '1'='1 escapes to 'x\' OR '1'='1'; the backslash is literal, the string closes early, and the tail executes. Watermark values are attacker-influenceable table data. Reachable but harder — quote-bearing values usually syntax-error the probe phase first (see below).
  • Quote-free data breaks too. stringMidpoint synthesizes bytes in [0x20,0x7E] as base95 digits — including 0x27 (') and 0x5C (\). Verified: stringMidpoint("&&&", "(((") = "'''", invented from quote-free input. The probe then throws ERROR 1064 and the build silently degrades to a single full-table partition.

Fix: use parameterized Execute(query, args...) (already supported at mysql.go:303) or '' quote-doubling instead of mysql.Escape. This fixes all three at once.

rs, err := c.Execute(ctx, query)
if err != nil {
return "", false, err
}
defer rs.Close()
if rs.RowNumber() == 0 {
return "", false, nil
}
key, err := rs.GetString(0, 0)
if err != nil {
return "", false, fmt.Errorf("failed to read next real key: %w", err)
}
// GetString is zero-copy over the resultset's row buffer, which rs.Close()
// recycles into a pool where the next query's response overwrites it; the
// key is stored in the heap and outlives the function, so it must own its bytes
return strings.Clone(key), true, nil
}

// estimateRowsInRange asks the query planner for the estimated number of rows in
// [start, end). The estimate is best-effort: a NULL or zero estimate is returned
// as 0 rather than an error, so the caller still treats the range as a real partition.
func (c *MySqlConnector) estimateRowsInRange(
ctx context.Context, tableName string, quotedCol string, start string, end string,
) (uint64, error) {
query := fmt.Sprintf(
"EXPLAIN FORMAT=TRADITIONAL SELECT 1 FROM %[1]s WHERE %[2]s >= '%[3]s' AND %[2]s < '%[4]s'",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without FORMAT=TRADITIONAL, on newer version of mysql it will return:

mysql> explain select 1 from str_test where id > 'a' and id <= 'z';
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| EXPLAIN                                                                                                                                                                                |
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| -> Filter: ((str_test.id > 'a') and (str_test.id <= 'z'))  (cost=18.9 rows=93)
    -> Covering index range scan on str_test using PRIMARY over ('a' < id <= 'z')  (cost=18.9 rows=93)
 |
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

what we want:

mysql> explain format=traditional select 1 from str_test where id > 'a' and id <= 'z';
+----+-------------+----------+------------+-------+---------------+---------+---------+------+------+----------+--------------------------+
| id | select_type | table    | partitions | type  | possible_keys | key     | key_len | ref  | rows | filtered | Extra                    |
+----+-------------+----------+------------+-------+---------------+---------+---------+------+------+----------+--------------------------+
|  1 | SIMPLE      | str_test | NULL       | range | PRIMARY       | PRIMARY | 202     | NULL |   93 |   100.00 | Using where; Using index |
+----+-------------+----------+------------+-------+---------------+---------+---------+------+------+----------+--------------------------+

tableName, quotedCol, mysql.Escape(start), mysql.Escape(end))
rs, err := c.Execute(ctx, query)
if err != nil {
return 0, err
}
defer rs.Close()
if rs.RowNumber() == 0 {
return 0, fmt.Errorf("no resultset for table %s", tableName)
}
rows, err := rs.GetUintByName(0, "rows")
if err != nil {
return 0, err
}
return rows, nil
}
Loading
Loading