Skip to content

Commit

Permalink
[#22195] YSQL: Unify ybgin and lsm logic
Browse files Browse the repository at this point in the history
Summary:
The ybgin and lsm access methods repeat a lot of logic during column binding. This diff deduplicates such logic. This diff also adds the `yb_is_supported` method to the IndexAMHandler interface to simplify areas of the code that check whether an index is supported by YB. This diff also adds the `yb_am_bind_schema` method the same interface. This method allows YB AM's to customize underlying DocDB schema creation and also pass along any extra metadata. Right now, the two YB AM's, yblsm and ybgin, implement the same logic for this method.

These changes are useful for upcoming changes where new index types are added by users or extensions, more specifically for the upcoming addition of a vector-based AM.
Jira: DB-11118

Test Plan: Jenkins

Reviewers: jason

Reviewed By: jason

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D34676
  • Loading branch information
tanujnay112 committed May 3, 2024
1 parent f3b9e49 commit 37203f5
Show file tree
Hide file tree
Showing 11 changed files with 192 additions and 125 deletions.
67 changes: 39 additions & 28 deletions src/postgres/src/backend/access/yb_access/yb_lsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "access/yb_scan.h"
#include "catalog/index.h"
#include "catalog/pg_type.h"
#include "commands/ybccmds.h"
#include "executor/ybcModifyTable.h"
#include "miscadmin.h"
#include "pgstat.h"
Expand All @@ -50,21 +51,6 @@ typedef struct
const uint64_t *backfill_write_time;
} YBCBuildState;

/*
* Utility method to bind const to column.
*/
static void
bindColumn(YBCPgStatement stmt,
int attr_num,
Oid type_id,
Oid collation_id,
Datum datum,
bool is_null)
{
YBCPgExpr expr = YBCNewConstant(stmt, type_id, collation_id, datum,
is_null);
HandleYBStatus(YBCPgDmlBindColumn(stmt, attr_num, expr));
}

/*
* Utility method to set binds for index write statement.
Expand Down Expand Up @@ -98,7 +84,14 @@ doBindsForIdxWrite(YBCPgStatement stmt,
Datum value = values[attnum - 1];
bool is_null = isnull[attnum - 1];

bindColumn(stmt, attnum, type_id, collation_id, value, is_null);
YbBindDatumToColumn(stmt,
attnum,
type_id,
collation_id,
value,
is_null,
NULL /* null_type_entity */);


/*
* If any of the indexed columns is null, we need to take case of
Expand All @@ -116,25 +109,28 @@ doBindsForIdxWrite(YBCPgStatement stmt,
* - to NULL otherwise (setting is_null to true is enough).
*/
if (unique_index)
bindColumn(stmt,
YBUniqueIdxKeySuffixAttributeNumber,
BYTEAOID,
InvalidOid,
ybbasectid,
!has_null_attr /* is_null */);
YbBindDatumToColumn(stmt,
YBUniqueIdxKeySuffixAttributeNumber,
BYTEAOID,
InvalidOid,
ybbasectid,
!has_null_attr /* is_null */,
NULL /* null_type_entity */);

/*
* We may need to set the base ctid column:
* - for unique indexes only if we need it as a value (i.e. for inserts)
* - for non-unique indexes always (it is a key column).
*/
if (ybctid_as_value || !unique_index)
bindColumn(stmt,
YBIdxBaseTupleIdAttributeNumber,
BYTEAOID,
InvalidOid,
ybbasectid,
false /* is_null */);
YbBindDatumToColumn(stmt,
YBIdxBaseTupleIdAttributeNumber,
BYTEAOID,
InvalidOid,
ybbasectid,
false /* is_null */,
NULL /* null_type_entity */);

}

static void
Expand Down Expand Up @@ -603,6 +599,19 @@ ybcinendscan(IndexScanDesc scan)
ybc_free_ybscan((YbScanDesc)scan->opaque);
}

static void
ybcinbindschema(YBCPgStatement handle,
struct IndexInfo *indexInfo,
TupleDesc indexTupleDesc,
int16 *coloptions)
{
YBCBindCreateIndexColumns(handle,
indexInfo,
indexTupleDesc,
coloptions,
indexInfo->ii_NumIndexKeyAttrs);
}

/*
* LSM handler function: return IndexAmRoutine with access method parameters
* and callbacks.
Expand Down Expand Up @@ -649,11 +658,13 @@ ybcinhandler(PG_FUNCTION_ARGS)
amroutine->amestimateparallelscan = yb_estimate_parallel_size;
amroutine->aminitparallelscan = yb_init_partition_key_data;
amroutine->amparallelrescan = NULL;
amroutine->yb_amisforybrelation = true;
amroutine->yb_aminsert = ybcininsert;
amroutine->yb_amdelete = ybcindelete;
amroutine->yb_ambackfill = ybcinbackfill;
amroutine->yb_ammightrecheck = ybcinmightrecheck;
amroutine->yb_amgetbitmap = ybcgetbitmap;
amroutine->yb_ambindschema = ybcinbindschema;

PG_RETURN_POINTER(amroutine);
}
32 changes: 32 additions & 0 deletions src/postgres/src/backend/access/yb_access/yb_scan.c
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,38 @@ ybcBindTupleExprCondIn(YbScanDesc ybScan,
ReleaseTupleDesc(tupdesc);
}

/*
* Utility method to bind const to column.
*/
void
YbBindDatumToColumn(YBCPgStatement stmt,
int attr_num,
Oid type_id,
Oid collation_id,
Datum datum,
bool is_null,
const YBCPgTypeEntity *null_type_entity)
{
YBCPgExpr expr;
const YBCPgTypeEntity *type_entity;

if (is_null && null_type_entity)
type_entity = null_type_entity;
else
type_entity = YbDataTypeFromOidMod(InvalidAttrNumber, type_id);

YBCPgCollationInfo collation_info;
YBGetCollationInfo(collation_id, type_entity, datum, is_null,
&collation_info);

HandleYBStatus(YBCPgNewConstant(stmt, type_entity,
collation_info.collate_is_valid_non_c,
collation_info.sortkey,
datum, is_null, &expr));

HandleYBStatus(YBCPgDmlBindColumn(stmt, attr_num, expr));
}

/*
* Add a system column as target to the given statement handle.
*/
Expand Down
2 changes: 2 additions & 0 deletions src/postgres/src/backend/access/ybgin/ybgin.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@ ybginhandler(PG_FUNCTION_ARGS)
amroutine->amestimateparallelscan = NULL;
amroutine->aminitparallelscan = NULL;
amroutine->amparallelrescan = NULL;
amroutine->yb_amisforybrelation = true;
amroutine->yb_aminsert = ybgininsert;
amroutine->yb_amdelete = ybgindelete;
amroutine->yb_ambackfill = ybginbackfill;
amroutine->yb_ammightrecheck = ybginmightrecheck;
amroutine->yb_ambindschema = ybginbindschema;

PG_RETURN_POINTER(amroutine);
}
15 changes: 15 additions & 0 deletions src/postgres/src/backend/access/ybgin/ybginutil.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
#include "access/gin_private.h"
#include "access/reloptions.h"
#include "c.h"
#include "nodes/execnodes.h"
#include "commands/ybccmds.h"
#include "nodes/relation.h"
#include "nodes/nodes.h"
#include "utils/index_selfuncs.h"
Expand Down Expand Up @@ -98,6 +100,19 @@ ybginvalidate(Oid opclassoid)
return ginvalidate(opclassoid);
}

void
ybginbindschema(YBCPgStatement handle,
struct IndexInfo *indexInfo,
TupleDesc indexTupleDesc,
int16 *coloptions)
{
YBCBindCreateIndexColumns(handle,
indexInfo,
indexTupleDesc,
coloptions,
indexInfo->ii_NumIndexKeyAttrs);
}

/*
* Given gin null category, return human-readable string.
*/
Expand Down
63 changes: 11 additions & 52 deletions src/postgres/src/backend/access/ybgin/ybginwrite.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include "access/genam.h"
#include "access/sysattr.h"
#include "access/yb_scan.h"
#include "access/ybgin_private.h"
#include "c.h"
#include "catalog/index.h"
Expand Down Expand Up @@ -58,51 +59,6 @@ typedef struct
uint64_t *backfilltime;
} YbginBuildState;

/*
* Utility method to create constant.
*/
static void
newConstant(YBCPgStatement stmt,
Oid type_id,
Oid collation_id,
Datum datum,
bool is_null,
YBCPgExpr *expr)
{
const YBCPgTypeEntity *type_entity;

if (is_null)
type_entity = &YBCGinNullTypeEntity;
else
type_entity = YbDataTypeFromOidMod(InvalidAttrNumber, type_id);

YBCPgCollationInfo collation_info;
YBGetCollationInfo(collation_id, type_entity, datum, is_null,
&collation_info);

HandleYBStatus(YBCPgNewConstant(stmt, type_entity,
collation_info.collate_is_valid_non_c,
collation_info.sortkey,
datum, is_null, expr));
}

/*
* Utility method to bind const to column.
*/
static void
bindColumn(YBCPgStatement stmt,
int attr_num,
Oid type_id,
Oid collation_id,
Datum datum,
bool is_null)
{
YBCPgExpr expr;

newConstant(stmt, type_id, collation_id, datum, is_null, &expr);
HandleYBStatus(YBCPgDmlBindColumn(stmt, attr_num, expr));
}

/*
* Utility method to set binds for index write statement.
*/
Expand Down Expand Up @@ -132,19 +88,22 @@ doBindsForIdxWrite(YBCPgStatement stmt,
Datum value = values[attnum - 1];
bool is_null = isnull[attnum - 1];

bindColumn(stmt, attnum, type_id, collation_id, value, is_null);
YbBindDatumToColumn(
stmt, attnum, type_id, collation_id,
value, is_null, &YBCGinNullTypeEntity);
}

/* Gin indexes cannot be unique. */
Assert(!index->rd_index->indisunique);

/* Write base ctid column because it is a key column. */
bindColumn(stmt,
YBIdxBaseTupleIdAttributeNumber,
BYTEAOID,
InvalidOid,
ybbasectid,
false /* is_null */);
YbBindDatumToColumn(stmt,
YBIdxBaseTupleIdAttributeNumber,
BYTEAOID,
InvalidOid,
ybbasectid,
false /* is_null */,
NULL /* null type_entity */);
}

/*
Expand Down
12 changes: 5 additions & 7 deletions src/postgres/src/backend/commands/indexcmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -941,24 +941,22 @@ DefineIndex(Oid relationId,
}
accessMethodId = HeapTupleGetOid(tuple);

if (IsYBRelation(rel) && (accessMethodId != LSM_AM_OID &&
accessMethodId != YBGIN_AM_OID))
accessMethodForm = (Form_pg_am) GETSTRUCT(tuple);
amRoutine = GetIndexAmRoutine(accessMethodForm->amhandler);

if (IsYBRelation(rel) && !amRoutine->yb_amisforybrelation)
ereport(ERROR,
(errmsg("index method \"%s\" not supported yet",
accessMethodName),
errhint("See https://github.com/yugabyte/yugabyte-db/issues/1337. "
"React with thumbs up to raise its priority")));
if (!IsYBRelation(rel) && (accessMethodId == LSM_AM_OID ||
accessMethodId == YBGIN_AM_OID))
if (!IsYBRelation(rel) && amRoutine->yb_amisforybrelation)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("access method \"%s\" only supported for indexes"
" using Yugabyte storage",
accessMethodName)));

accessMethodForm = (Form_pg_am) GETSTRUCT(tuple);
amRoutine = GetIndexAmRoutine(accessMethodForm->amhandler);

if (stmt->unique && !amRoutine->amcanunique)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
Expand Down

0 comments on commit 37203f5

Please sign in to comment.