[PATCH 0/3] some SQLite-related things

[PATCH 0/3] some SQLite-related things

[Eric Wong]

The first two are a bit shocking :x, but seem to work alright
forwards and backwards compatibility-wise for more correct
code.  3/3 is straightforward, at least.

Eric Wong (3):
  sqlite: avoid incorrect/deprecated `LIKE' use
  sqlite: use `BLOB' column type instead of `VARBINARY'
  SQLiteUtil: hoist out common create_db code

 MANIFEST                        |  2 ++
 lib/PublicInbox/ExtSearchIdx.pm | 12 ++++----
 lib/PublicInbox/IMAPTracker.pm  |  5 ++--
 lib/PublicInbox/LeiMailSync.pm  | 28 ++++++-------------
 lib/PublicInbox/Over.pm         |  9 ++----
 lib/PublicInbox/OverIdx.pm      |  4 +--
 lib/PublicInbox/POP3D.pm        | 12 +++-----
 lib/PublicInbox/SQLiteUtil.pm   | 40 +++++++++++++++++++++++++++
 lib/PublicInbox/SharedKV.pm     | 33 ++++++++--------------
 t/shared_kv.t                   |  5 ++++
 t/sqlite_util.t                 | 49 +++++++++++++++++++++++++++++++++
 11 files changed, 134 insertions(+), 65 deletions(-)
 create mode 100644 lib/PublicInbox/SQLiteUtil.pm
 create mode 100644 t/sqlite_util.t

[PATCH 1/3] sqlite: avoid incorrect/deprecated `LIKE' use

[Eric Wong]

The `case_sensitive_like' pragma is deprecated since
SQLite 3.44+.  Furthermore, our use of `LIKE' in SharedKV->keys
seemed to be broken anyways since `LIKE' doesn't seem to
work with binary data (stored with SQL_BLOB), but neither does
`GLOB'.

So avoid `LIKE' entirely.  For non-SQL_BLOB data we'll favor the
always-case-sensitive GLOB.  For SQL_BLOB data, we must rely on
the Perl regexp engine from what I can tell.  `GLOB' is
preferred where possible since SQLite will be able to use
indices in some cases whereas `REGEXP' cannot.

Fixing SharedKV->keys should improve bash completion for lei.

Some common SQLite-related utilities are now in a
PublicInbox::SQLiteUtil package which will be expanded to deal
with more commonalities between SQLite users in our tree.
---
 MANIFEST                        |  2 ++
 lib/PublicInbox/ExtSearchIdx.pm | 12 ++++----
 lib/PublicInbox/LeiMailSync.pm  | 14 +++-------
 lib/PublicInbox/SQLiteUtil.pm   | 30 ++++++++++++++++++++
 lib/PublicInbox/SharedKV.pm     | 23 ++++++----------
 t/shared_kv.t                   |  5 ++++
 t/sqlite_util.t                 | 49 +++++++++++++++++++++++++++++++++
 7 files changed, 105 insertions(+), 30 deletions(-)
 create mode 100644 lib/PublicInbox/SQLiteUtil.pm
 create mode 100644 t/sqlite_util.t

diff --git a/MANIFEST b/MANIFEST
index 1305ed8a..b0b4f71c 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -334,6 +334,7 @@ lib/PublicInbox/RepoList.pm
 lib/PublicInbox/RepoSnapshot.pm
 lib/PublicInbox/RepoTree.pm
 lib/PublicInbox/SHA.pm
+lib/PublicInbox/SQLiteUtil.pm
 lib/PublicInbox/SaPlugin/ListMirror.pm
 lib/PublicInbox/SaPlugin/ListMirror.pod
 lib/PublicInbox/Search.pm
@@ -614,6 +615,7 @@ t/solve/bare.patch
 t/solver_git.t
 t/spamcheck_spamc.t
 t/spawn.t
+t/sqlite_util.t
 t/syscall.t
 t/tail_notify.t
 t/thread-cycle.t
diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index cead0f8a..d8db7d4b 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -21,6 +21,7 @@ use Carp qw(croak carp);
 use Scalar::Util qw(blessed);
 use Sys::Hostname qw(hostname);
 use File::Glob qw(bsd_glob GLOB_NOSORT);
+use PublicInbox::SQLiteUtil;
 use PublicInbox::Isearch;
 use PublicInbox::MultiGit;
 use PublicInbox::Spawn ();
@@ -461,15 +462,14 @@ EOM
 DELETE FROM inboxes WHERE ibx_id = ?
 
 		# drop last_commit info
-		my $pat = $eidx_key;
-		$pat =~ s/([_%\\])/\\$1/g;
-		$self->{oidx}->dbh->do('PRAGMA case_sensitive_like = ON');
+		# We use GLOB in addition to REGEXP since GLOB can use indices
 		my $lc_i = $self->{oidx}->dbh->prepare(<<'');
-SELECT key FROM eidx_meta WHERE key LIKE ? ESCAPE ?
+SELECT key FROM eidx_meta WHERE key GLOB ? AND key REGEXP ?
 
-		$lc_i->execute("lc-%:$pat//%", '\\');
+		my $ekg = 'lc-v[1-9]*:'.
+			PublicInbox::SQLiteUtil::escape_glob($eidx_key).'//*';
+		$lc_i->execute($ekg, qr!\Alc-v[1-9]+:\Q$eidx_key\E//!);
 		while (my ($key) = $lc_i->fetchrow_array) {
-			next if $key !~ m!\Alc-v[1-9]+:\Q$eidx_key\E//!;
 			warn "# removing $key\n";
 			$self->{oidx}->dbh->do(<<'', undef, $key);
 DELETE FROM eidx_meta WHERE key = ?
diff --git a/lib/PublicInbox/LeiMailSync.pm b/lib/PublicInbox/LeiMailSync.pm
index d0f6d7b4..d23f6b4b 100644
--- a/lib/PublicInbox/LeiMailSync.pm
+++ b/lib/PublicInbox/LeiMailSync.pm
@@ -12,6 +12,7 @@ use PublicInbox::ContentHash qw(git_sha);
 use Carp ();
 use PublicInbox::Git qw(%HEXLEN2SHA);
 use PublicInbox::IO qw(read_all);
+use PublicInbox::SQLiteUtil;
 
 sub dbh_new {
 	my ($self) = @_;
@@ -31,7 +32,6 @@ sub dbh_new {
 	# no sqlite_unicode, here, all strings are binary
 	create_tables($self, $dbh);
 	$dbh->do('PRAGMA journal_mode = WAL') if $creat;
-	$dbh->do('PRAGMA case_sensitive_like = ON');
 	$dbh;
 }
 
@@ -421,18 +421,12 @@ sub locations_for {
 
 # returns a list of folders used for completion
 sub folders {
-	my ($self, @pfx) = @_;
+	my ($self, $pfx, $anywhere) = @_;
 	my $sql = 'SELECT loc FROM folders';
 	my $re;
-	if (defined($pfx[0])) {
+	if (defined $pfx) {
 		$sql .= ' WHERE loc REGEXP ?'; # DBD::SQLite uses perlre
-		if (ref($pfx[0])) { # assume qr// "Regexp"
-			$re = $pfx[0];
-		} else {
-			$re = !!$pfx[1] ? '.*' : '';
-			$re .= quotemeta($pfx[0]);
-			$re .= '.*';
-		}
+		$re = PublicInbox::SQLiteUtil::mk_sqlite_re $pfx, $anywhere;
 	}
 	my $sth = ($self->{dbh} //= dbh_new($self))->prepare($sql);
 	$sth->bind_param(1, $re) if defined($re);
diff --git a/lib/PublicInbox/SQLiteUtil.pm b/lib/PublicInbox/SQLiteUtil.pm
new file mode 100644
index 00000000..68e0726d
--- /dev/null
+++ b/lib/PublicInbox/SQLiteUtil.pm
@@ -0,0 +1,30 @@
+# Copyright (C) all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+
+# common bits for SQLite users in our codebase
+package PublicInbox::SQLiteUtil;
+use v5.12;
+
+my %SQLITE_GLOB_MAP = (
+	'[' => '[[]',
+	']' => '[]]',
+	'*' => '[*]',
+	'?' => '[?]'
+);
+
+# n.b. GLOB doesn't seem to work on data inserted w/ SQL_BLOB
+sub escape_glob ($) {
+	my ($s) = @_;
+	$s =~ s/([\[\]\*\?])/$SQLITE_GLOB_MAP{$1}/sge;
+	$s;
+}
+
+# DBD::SQLite maps REGEXP to use perlre, and that works on SQL_BLOB
+# whereas GLOB and LIKE don't seem to...
+sub mk_sqlite_re ($$) {
+	my ($pfx, $anywhere) = @_;
+	ref($pfx) ? $pfx # assume qr// Regexp
+		: ($anywhere ? '.*' : '^')."\Q$pfx\E.*";
+}
+
+1;
diff --git a/lib/PublicInbox/SharedKV.pm b/lib/PublicInbox/SharedKV.pm
index 89ab3f74..51ece48d 100644
--- a/lib/PublicInbox/SharedKV.pm
+++ b/lib/PublicInbox/SharedKV.pm
@@ -12,6 +12,7 @@ use File::Temp qw(tempdir);
 use DBI qw(:sql_types); # SQL_BLOB
 use PublicInbox::Spawn;
 use File::Path qw(rmtree);
+use PublicInbox::SQLiteUtil;
 
 sub dbh {
 	my ($self, $lock) = @_;
@@ -79,23 +80,17 @@ SELECT k,v FROM kv
 }
 
 sub keys {
-	my ($self, @pfx) = @_;
+	my ($self, $pfx, $anywhere) = @_;
+	# n.b. can't use GLOB for index optimization due to SQL_BLOB,
+	# so regexps it is.
 	my $sql = 'SELECT k FROM kv';
-	if (defined $pfx[0]) {
-		$sql .= ' WHERE k LIKE ? ESCAPE ?';
-		my $anywhere = !!$pfx[1];
-		$pfx[1] = '\\';
-		$pfx[0] =~ s/([%_\\])/\\$1/g; # glob chars
-		$pfx[0] .= '%';
-		substr($pfx[0], 0, 0, '%') if $anywhere;
-	} else {
-		@pfx = (); # [0] may've been undef
+	my $re;
+	if (defined $pfx) {
+		$sql .= ' WHERE k REGEXP ?'; # DBD::SQLite uses perlre
+		$re = PublicInbox::SQLiteUtil::mk_sqlite_re $pfx, $anywhere;
 	}
 	my $sth = $self->dbh->prepare($sql);
-	if (@pfx) {
-		$sth->bind_param(1, $pfx[0], SQL_BLOB);
-		$sth->bind_param(2, $pfx[1]);
-	}
+	$sth->bind_param(1, $re) if defined $re;
 	$sth->execute;
 	map { $_->[0] } @{$sth->fetchall_arrayref};
 }
diff --git a/t/shared_kv.t b/t/shared_kv.t
index 8dfd3b25..aa1ce4e3 100644
--- a/t/shared_kv.t
+++ b/t/shared_kv.t
@@ -46,4 +46,9 @@ $skv->dbh;
 ok($skv->set_maybe('02', '2'), "`02' set");
 ok($skv->set_maybe('2', '2'), "`2' set (no match on `02')");
 
+my @k = $skv->keys('2');
+is_deeply \@k, [ '2' ], 'prefix match on ->keys';
+@k = sort $skv->keys('2', 1);
+is_deeply \@k, [ '02', '2' ], 'anywhere match on ->keys';
+
 done_testing;
diff --git a/t/sqlite_util.t b/t/sqlite_util.t
new file mode 100644
index 00000000..f41285e0
--- /dev/null
+++ b/t/sqlite_util.t
@@ -0,0 +1,49 @@
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use v5.12;
+use PublicInbox::TestCommon;
+require_mods 'DBD::SQLite';
+use_ok 'PublicInbox::SQLiteUtil';
+require DBI;
+DBI->import(':sql_types');
+
+my $dbh = DBI->connect('dbi:SQLite:dbname=:memory:', '', '', {
+	AutoCommit => 1,
+	RaiseError => 1,
+	PrintError => 0,
+	sqlite_use_immediate_transaction => 1,
+});
+
+$dbh->do('CREATE TABLE test (key BLOB NOT NULL, UNIQUE (key))');
+
+my $ins = $dbh->prepare('INSERT INTO test (key) VALUES (?)');
+my $sel = $dbh->prepare('SELECT key FROM test WHERE key GLOB ?');
+my $non_utf8 = "h\x{e5}llo[wor]ld!";
+my $us_ascii = 'h*llo[wor]ld?';
+
+$dbh->begin_work;
+my @SQL_BLOB = (SQL_BLOB());
+@SQL_BLOB = (); # FIXME: can't get GLOB to work w/ SQL_BLOB
+for my $k ($us_ascii, $non_utf8) {
+	$ins->bind_param(1, $k, @SQL_BLOB);
+	$ins->execute;
+}
+$dbh->commit;
+
+$sel->bind_param(1, '*', @SQL_BLOB);
+$sel->execute;
+my $rows = $sel->fetchall_arrayref;
+is scalar(@$rows), 2, q[`*' got everything];
+
+$sel->bind_param(1, PublicInbox::SQLiteUtil::escape_glob($us_ascii), @SQL_BLOB);
+$sel->execute;
+$rows = $sel->fetchall_arrayref;
+is_deeply $rows, [ [ $us_ascii ] ], 'US-ASCII exact match';
+
+$sel->bind_param(1, PublicInbox::SQLiteUtil::escape_glob($non_utf8), @SQL_BLOB);
+$sel->execute;
+$rows = $sel->fetchall_arrayref;
+is_deeply $rows, [ [ $non_utf8 ] ], 'ISO-8859-1 exact match';
+
+done_testing;

[PATCH 2/3] sqlite: use `BLOB' column type instead of `VARBINARY'

[Eric Wong]

`VARBINARY' isn't actually a documented type for SQLite but
rather the result of MySQL infecting my mind decades ago.
`BLOB' gives it the proper affinity and probably makes it easier
for 3rd-party and one-off scripts to deal with such columns and
should also make things more familiar to existing users.
Surprisingly, this appears to have no functional change with
forward or backwards compatibility since we ->bind_param binary
data with SQL_BLOB on INSERTs anyways to prevent the flexible
typing of SQLite from trying to guess types for us.
---
 lib/PublicInbox/LeiMailSync.pm | 8 ++++----
 lib/PublicInbox/OverIdx.pm     | 4 ++--
 lib/PublicInbox/POP3D.pm       | 4 ++--
 lib/PublicInbox/SharedKV.pm    | 4 ++--
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/PublicInbox/LeiMailSync.pm b/lib/PublicInbox/LeiMailSync.pm
index d23f6b4b..fc7963a1 100644
--- a/lib/PublicInbox/LeiMailSync.pm
+++ b/lib/PublicInbox/LeiMailSync.pm
@@ -61,13 +61,13 @@ sub create_tables {
 	$dbh->do(<<'');
 CREATE TABLE IF NOT EXISTS folders (
 	fid INTEGER PRIMARY KEY,
-	loc VARBINARY NOT NULL, /* URL;UIDVALIDITY=$N or $TYPE:/pathname */
+	loc BLOB NOT NULL, /* URL;UIDVALIDITY=$N or $TYPE:/pathname */
 	UNIQUE (loc)
 )
 
 	$dbh->do(<<'');
 CREATE TABLE IF NOT EXISTS blob2num (
-	oidbin VARBINARY NOT NULL,
+	oidbin BLOB NOT NULL,
 	fid INTEGER NOT NULL, /* folder ID */
 	uid INTEGER NOT NULL, /* NNTP article number, IMAP UID, MH number */
 	/* not UNIQUE(fid, uid), since we may have broken servers */
@@ -80,9 +80,9 @@ CREATE INDEX IF NOT EXISTS idx_fid_uid ON blob2num(fid,uid)
 
 	$dbh->do(<<'');
 CREATE TABLE IF NOT EXISTS blob2name (
-	oidbin VARBINARY NOT NULL,
+	oidbin BLOB NOT NULL,
 	fid INTEGER NOT NULL, /* folder ID */
-	name VARBINARY NOT NULL, /* Maildir basename, JMAP blobId */
+	name BLOB NOT NULL, /* Maildir basename, JMAP blobId */
 	/* not UNIQUE(fid, name), since we may have broken software */
 	UNIQUE (oidbin, fid, name)
 )
diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm
index 10cf8c39..dea6f0fc 100644
--- a/lib/PublicInbox/OverIdx.pm
+++ b/lib/PublicInbox/OverIdx.pm
@@ -389,7 +389,7 @@ CREATE TABLE IF NOT EXISTS over (
 	sid INTEGER, /* Subject ID (IMAP ORDEREDSUBJECT "threading") */
 	ts INTEGER, /* IMAP INTERNALDATE (Received: header, git commit time) */
 	ds INTEGER, /* RFC-2822 sent Date: header, git author time */
-	ddd VARBINARY /* doc-data-deflated (->to_doc_data, ->load_from_data) */
+	ddd BLOB /* doc-data-deflated (->to_doc_data, ->load_from_data) */
 )
 
 	$dbh->do('CREATE INDEX IF NOT EXISTS idx_tid ON over (tid)');
@@ -543,7 +543,7 @@ CREATE TABLE IF NOT EXISTS xref3 (
 	docid INTEGER NOT NULL, /* <=> over.num */
 	ibx_id INTEGER NOT NULL, /* <=> inboxes.ibx_id */
 	xnum INTEGER NOT NULL, /* NNTP article number in ibx */
-	oidbin VARBINARY NOT NULL, /* 20-byte SHA-1 or 32-byte SHA-256 */
+	oidbin BLOB NOT NULL, /* 20-byte SHA-1 or 32-byte SHA-256 */
 	UNIQUE (docid, ibx_id, xnum, oidbin)
 )
 
diff --git a/lib/PublicInbox/POP3D.pm b/lib/PublicInbox/POP3D.pm
index bd440434..1898c89d 100644
--- a/lib/PublicInbox/POP3D.pm
+++ b/lib/PublicInbox/POP3D.pm
@@ -93,7 +93,7 @@ sub create_state_tables ($$) {
 	$dbh->do(<<''); # map publicinbox.<name>.newsgroup to integers
 CREATE TABLE IF NOT EXISTS newsgroups (
 	newsgroup_id INTEGER PRIMARY KEY NOT NULL,
-	newsgroup VARBINARY NOT NULL,
+	newsgroup BLOB NOT NULL,
 	UNIQUE (newsgroup) )
 
 	# the $NEWSGROUP_NAME.$SLICE_INDEX is part of the POP3 username;
@@ -108,7 +108,7 @@ CREATE TABLE IF NOT EXISTS mailboxes (
 	$dbh->do(<<''); # actual users are differentiated by their UUID
 CREATE TABLE IF NOT EXISTS users (
 	user_id INTEGER PRIMARY KEY NOT NULL,
-	uuid VARBINARY NOT NULL,
+	uuid BLOB NOT NULL,
 	last_seen INTEGER NOT NULL, /* to expire idle accounts */
 	UNIQUE (uuid) )
 
diff --git a/lib/PublicInbox/SharedKV.pm b/lib/PublicInbox/SharedKV.pm
index 51ece48d..062d5e3e 100644
--- a/lib/PublicInbox/SharedKV.pm
+++ b/lib/PublicInbox/SharedKV.pm
@@ -32,8 +32,8 @@ sub dbh {
 				($opt->{journal_mode} // 'WAL'));
 		$dbh->do(<<'');
 CREATE TABLE IF NOT EXISTS kv (
-	k VARBINARY PRIMARY KEY NOT NULL,
-	v VARBINARY NOT NULL,
+	k BLOB PRIMARY KEY NOT NULL,
+	v BLOB NOT NULL,
 	UNIQUE (k)
 )
 

[PATCH 3/3] SQLiteUtil: hoist out common create_db code

[Eric Wong]

We want all SQLite files we create to respect the current umask
and disable CoW for random access performance.
---
 lib/PublicInbox/IMAPTracker.pm |  5 ++---
 lib/PublicInbox/LeiMailSync.pm |  6 +-----
 lib/PublicInbox/Over.pm        |  9 +++------
 lib/PublicInbox/POP3D.pm       |  8 ++------
 lib/PublicInbox/SQLiteUtil.pm  | 10 ++++++++++
 lib/PublicInbox/SharedKV.pm    |  6 +-----
 6 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/lib/PublicInbox/IMAPTracker.pm b/lib/PublicInbox/IMAPTracker.pm
index 4efa8a7e..2dd809b5 100644
--- a/lib/PublicInbox/IMAPTracker.pm
+++ b/lib/PublicInbox/IMAPTracker.pm
@@ -6,6 +6,7 @@ use parent qw(PublicInbox::Lock);
 use DBI;
 use DBD::SQLite;
 use PublicInbox::Config;
+use PublicInbox::SQLiteUtil;
 
 sub create_tables ($) {
 	my ($dbh) = @_;
@@ -75,11 +76,9 @@ sub new {
 	}
 	if (!-f $dbname) {
 		require File::Path;
-		require PublicInbox::Syscall;
 		my ($dir) = ($dbname =~ m!(.*?/)[^/]+\z!);
 		File::Path::mkpath($dir);
-		PublicInbox::Syscall::nodatacow_dir($dir);
-		open my $fh, '+>>', $dbname or die "failed to open $dbname: $!";
+		PublicInbox::SQLiteUtil::create_db $dbname;
 	}
 	my $self = bless { lock_path => "$dbname.lock", url => $url }, $class;
 	$self->lock_acquire;
diff --git a/lib/PublicInbox/LeiMailSync.pm b/lib/PublicInbox/LeiMailSync.pm
index fc7963a1..cab5bbb3 100644
--- a/lib/PublicInbox/LeiMailSync.pm
+++ b/lib/PublicInbox/LeiMailSync.pm
@@ -18,11 +18,7 @@ sub dbh_new {
 	my ($self) = @_;
 	my $f = $self->{filename};
 	my $creat = !-s $f;
-	if ($creat) {
-		require PublicInbox::Syscall;
-		open my $fh, '+>>', $f or Carp::croak "open($f): $!";
-		PublicInbox::Syscall::nodatacow_fh($fh);
-	}
+	PublicInbox::SQLiteUtil::create_db $f if $creat;
 	my $dbh = DBI->connect("dbi:SQLite:dbname=$f",'','', {
 		AutoCommit => 1,
 		RaiseError => 1,
diff --git a/lib/PublicInbox/Over.pm b/lib/PublicInbox/Over.pm
index ff5332e7..0ecdae3b 100644
--- a/lib/PublicInbox/Over.pm
+++ b/lib/PublicInbox/Over.pm
@@ -13,17 +13,14 @@ use PublicInbox::Smsg;
 use Compress::Zlib qw(uncompress);
 use constant DEFAULT_LIMIT => 1000;
 use List::Util (); # for max
-use autodie qw(open);
+use PublicInbox::SQLiteUtil;
 
 sub dbh_new {
 	my ($self, $rw) = @_;
 	my $f = delete $self->{filename};
-	if (!-s $f) { # SQLite defaults mode to 0644, we want 0666
+	if (!-s $f) {
 		if ($rw) {
-			require PublicInbox::Syscall;
-			my ($dir) = ($f =~ m!(.+)/[^/]+\z!);
-			PublicInbox::Syscall::nodatacow_dir($dir);
-			open my $fh, '+>>', $f;
+			PublicInbox::SQLiteUtil::create_db $f;
 		} else {
 			$self->{filename} = $f; # die on stat() below:
 		}
diff --git a/lib/PublicInbox/POP3D.pm b/lib/PublicInbox/POP3D.pm
index 1898c89d..a30fc677 100644
--- a/lib/PublicInbox/POP3D.pm
+++ b/lib/PublicInbox/POP3D.pm
@@ -10,7 +10,7 @@ use Carp ();
 use File::Temp 0.19 (); # 0.19 for ->newdir
 use PublicInbox::Config;
 use PublicInbox::POP3;
-use PublicInbox::Syscall;
+use PublicInbox::SQLiteUtil;
 use File::Temp 0.19 (); # 0.19 for ->newdir
 use Fcntl qw(F_SETLK F_UNLCK F_WRLCK SEEK_SET);
 my ($FLOCK_TMPL, @FLOCK_ORDER);
@@ -73,7 +73,6 @@ sub refresh_groups { # PublicInbox::Daemon callback
 	-d $d or do {
 		require File::Path;
 		File::Path::make_path($d, { mode => 0700 });
-		PublicInbox::Syscall::nodatacow_dir($d);
 	};
 	$self->{lock_path} //= "$d/db.lock";
 	if (my $old = $self->{pi_cfg}) {
@@ -127,10 +126,7 @@ sub state_dbh_new {
 	my ($self) = @_;
 	my $f = "$self->{pi_cfg}->{'publicinbox.pop3state'}/db.sqlite3";
 	my $creat = !-s $f;
-	if ($creat) {
-		open my $fh, '+>>', $f or Carp::croak "open($f): $!";
-		PublicInbox::Syscall::nodatacow_fh($fh);
-	}
+	PublicInbox::SQLiteUtil::create_db $f if $creat;
 
 	my $dbh = DBI->connect("dbi:SQLite:dbname=$f",'','', {
 		AutoCommit => 1,
diff --git a/lib/PublicInbox/SQLiteUtil.pm b/lib/PublicInbox/SQLiteUtil.pm
index 68e0726d..fcec9e4c 100644
--- a/lib/PublicInbox/SQLiteUtil.pm
+++ b/lib/PublicInbox/SQLiteUtil.pm
@@ -4,6 +4,7 @@
 # common bits for SQLite users in our codebase
 package PublicInbox::SQLiteUtil;
 use v5.12;
+use autodie qw(open);
 
 my %SQLITE_GLOB_MAP = (
 	'[' => '[[]',
@@ -27,4 +28,13 @@ sub mk_sqlite_re ($$) {
 		: ($anywhere ? '.*' : '^')."\Q$pfx\E.*";
 }
 
+sub create_db ($) {
+	my ($f) = @_;
+	require PublicInbox::Syscall;
+	my ($dir) = ($f =~ m!(.+)/[^/]+\z!);
+	PublicInbox::Syscall::nodatacow_dir($dir); # for journal/shm/wal
+	# SQLite defaults mode to 0644, we want 0666 to respect umask
+	open my $fh, '+>>', $f;
+}
+
 1;
diff --git a/lib/PublicInbox/SharedKV.pm b/lib/PublicInbox/SharedKV.pm
index 062d5e3e..3aafff50 100644
--- a/lib/PublicInbox/SharedKV.pm
+++ b/lib/PublicInbox/SharedKV.pm
@@ -49,11 +49,7 @@ sub new {
 	$base //= '';
 	my $f = $self->{filename} = "$dir/$base.sqlite3";
 	$self->{lock_path} = $opt->{lock_path} // "$dir/$base.flock";
-	unless (-s $f) {
-		require PublicInbox::Syscall;
-		PublicInbox::Syscall::nodatacow_dir($dir); # for journal/shm/wal
-		open my $fh, '+>>', $f or die "failed to open $f: $!";
-	}
+	PublicInbox::SQLiteUtil::create_db $f if !-s $f;
 	$self;
 }