Extra options to lookup() which get passed to insert()

[simonw]

For simonw/git-history#12 I found myself wanting to pass extra options to lookup() to set the column order, primary key etc.

[simonw]

Looking at the code for lookup() it currently hard-codes pk to "id" - but it actually only calls .insert() in two places, both of which could be passed extra arguments.

sqlite-utils/sqlite_utils/db.py

Lines 2756 to 2763 in 3b8abe6

	try:
	return rows[0]["id"]
	except IndexError:
	return self.insert(combined_values, pk="id").last_pk
	else:
	pk = self.insert(combined_values, pk="id").last_pk
	self.create_index(lookup_values.keys(), unique=True)
	return pk

[simonw]

pk needs to be an explicit argument to .lookup(). The rest could be **kwargs passed through to .insert(), like this hacked together version (docstring removed for brevity):

    def lookup(
        self,
        lookup_values: Dict[str, Any],
        extra_values: Optional[Dict[str, Any]] = None,
        pk="id",
        **insert_kwargs,
    ):
        """
        assert isinstance(lookup_values, dict)
        if extra_values is not None:
            assert isinstance(extra_values, dict)
        combined_values = dict(lookup_values)
        if extra_values is not None:
            combined_values.update(extra_values)
        if self.exists():
            self.add_missing_columns([combined_values])
            unique_column_sets = [set(i.columns) for i in self.indexes]
            if set(lookup_values.keys()) not in unique_column_sets:
                self.create_index(lookup_values.keys(), unique=True)
            wheres = ["[{}] = ?".format(column) for column in lookup_values]
            rows = list(
                self.rows_where(
                    " and ".join(wheres), [value for _, value in lookup_values.items()]
                )
            )
            try:
                return rows[0][pk]
            except IndexError:
                return self.insert(combined_values, pk=pk, **insert_kwargs).last_pk
        else:
            pk = self.insert(combined_values, pk=pk, **insert_kwargs).last_pk
            self.create_index(lookup_values.keys(), unique=True)
            return pk

I think I'll explicitly list the parameters, mainly so they can be typed and covered by automatic documentation.

I do worry that I'll add more keyword arguments to .insert() in the future and forget to mirror them to .lookup() though.

[simonw]

Here's the current full method signature for .insert():

sqlite-utils/sqlite_utils/db.py

Lines 2462 to 2477 in 3b8abe6

	def insert(
	self,
	record: Dict[str, Any],
	pk=DEFAULT,
	foreign_keys=DEFAULT,
	column_order: Optional[Union[List[str], Default]] = DEFAULT,
	not_null: Optional[Union[Set[str], Default]] = DEFAULT,
	defaults: Optional[Union[Dict[str, Any], Default]] = DEFAULT,
	hash_id: Optional[Union[str, Default]] = DEFAULT,
	alter: Optional[Union[bool, Default]] = DEFAULT,
	ignore: Optional[Union[bool, Default]] = DEFAULT,
	replace: Optional[Union[bool, Default]] = DEFAULT,
	extracts: Optional[Union[Dict[str, str], List[str], Default]] = DEFAULT,
	conversions: Optional[Union[Dict[str, str], Default]] = DEFAULT,
	columns: Optional[Union[Dict[str, Any], Default]] = DEFAULT,
	) -> "Table":

I could add a test which uses introspection (inspect.signature(method).parameters) to confirm that .lookup() has a super-set of the arguments accepted by .insert().

[simonw]

Also: I don't think ignore= and replace= make sense in the context of lookup().

[simonw]

And neither does hash_id. On that basis I'm going to specifically list the ones that DO make sense, and hope that I remember to add any new ones in the future. I can add a code comment hint to .insert() about that.

[simonw]

I don't think I need the DEFAULT defaults for .insert() either, since it just passes through to .insert().

[simonw]

alter=True doesn't make sense to support here either, because .lookup() already adds missing columns:

sqlite-utils/sqlite_utils/db.py

Lines 2743 to 2746 in 3b8abe6

	if extra_values is not None:
	combined_values.update(extra_values)
	if self.exists():
	self.add_missing_columns([combined_values])