Skip to content

Commit

Permalink
fixed missed KNN options for adding attribute at alter table; added c…
Browse files Browse the repository at this point in the history
…ase to test 275; fixed #2230
  • Loading branch information
tomatolog authored and sanikolaev committed Jun 25, 2024
1 parent d1308c5 commit 3d5014e
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/searchd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16122,6 +16122,7 @@ static void AddAttrToIndex ( const SqlStmt_t & tStmt, CSphIndex * pIdx, CSphStri
tCtx.m_iBits = tStmt.m_iBits;
tCtx.m_uFlags = tStmt.m_uAttrFlags;
tCtx.m_eEngine = tStmt.m_eEngine;
tCtx.m_tKNN = tStmt.m_tAlterKNN;

if ( bIndexed || bStored )
{
Expand Down
27 changes: 20 additions & 7 deletions src/searchdddl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class DdlParser_c : public SqlParserTraits_c

void Reset() { *this = ItemOptions_t(); }
DWORD ToFlags() const;
knn::IndexSettings_t ToKNN() const;
};

DdlParser_c ( CSphVector<SqlStmt_t>& dStmt, const char* szQuery, CSphString* pError );
Expand Down Expand Up @@ -127,6 +128,19 @@ DWORD DdlParser_c::ItemOptions_t::ToFlags() const
return uFlags;
}

knn::IndexSettings_t DdlParser_c::ItemOptions_t::ToKNN() const
{
knn::IndexSettings_t tKNN;

tKNN.m_iDims = m_iKNNDims;
tKNN.m_eHNSWSimilarity = m_eHNSWSimilarity;
tKNN.m_iHNSWM = m_iHNSWM;
tKNN.m_iHNSWEFConstruction = m_iHNSWEFConstruction;

return tKNN;

}

//////////////////////////////////////////////////////////////////////////

DdlParser_c::DdlParser_c ( CSphVector<SqlStmt_t> & dStmt, const char* szQuery, CSphString* pError )
Expand Down Expand Up @@ -225,7 +239,6 @@ bool DdlParser_c::CheckFieldFlags ( ESphAttr eAttrType, int iFlags, const CSphSt
bool DdlParser_c::SetupAlterTable ( const SqlNode_t & tIndex, const SqlNode_t & tAttr, ESphAttr eAttr, int iFieldFlags, int iBits, bool bModify )
{
assert( m_pStmt );
ItemOptions_t tOpts = m_tItemOptions;

m_pStmt->m_eStmt = bModify ? STMT_ALTER_MODIFY : STMT_ALTER_ADD;
ToString ( m_pStmt->m_sIndex, tIndex );
Expand All @@ -235,11 +248,14 @@ bool DdlParser_c::SetupAlterTable ( const SqlNode_t & tIndex, const SqlNode_t &
m_pStmt->m_eAlterColType = eAttr;
m_pStmt->m_uFieldFlags = ConvertFlags(iFieldFlags);
m_pStmt->m_uAttrFlags = m_tItemOptions.ToFlags();
m_pStmt->m_eEngine = tOpts.m_eEngine;
m_pStmt->m_eEngine = m_tItemOptions.m_eEngine;
m_pStmt->m_iBits = iBits;
m_pStmt->m_tAlterKNN = m_tItemOptions.ToKNN();

bool bOk = CheckFieldFlags ( m_pStmt->m_eAlterColType, iFieldFlags, m_pStmt->m_sAlterAttr, m_tItemOptions, m_sError );
m_tItemOptions.Reset();

return CheckFieldFlags ( m_pStmt->m_eAlterColType, iFieldFlags, m_pStmt->m_sAlterAttr, tOpts, m_sError );
return bOk;
}


Expand Down Expand Up @@ -275,10 +291,7 @@ bool DdlParser_c::AddCreateTableCol ( const SqlNode_t & tName, const SqlNode_t &
tAttr.m_bFastFetch = tOpts.m_bFastFetch;
tAttr.m_bStringHash = tOpts.m_bStringHash;
tAttr.m_bKNN = !tOpts.m_sKNNType.IsEmpty();
tAttr.m_tKNN.m_iDims = tOpts.m_iKNNDims;
tAttr.m_tKNN.m_eHNSWSimilarity = tOpts.m_eHNSWSimilarity;
tAttr.m_tKNN.m_iHNSWM = tOpts.m_iHNSWM;
tAttr.m_tKNN.m_iHNSWEFConstruction = tOpts.m_iHNSWEFConstruction;
tAttr.m_tKNN = tOpts.ToKNN();

return true;
}
Expand Down
1 change: 1 addition & 0 deletions src/searchdsql.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ struct SqlStmt_t
DWORD m_uFieldFlags = 0;
DWORD m_uAttrFlags = 0;
int m_iBits = -1;
knn::IndexSettings_t m_tAlterKNN;

// CREATE TABLE specific
CreateTableSettings_t m_tCreateTable;
Expand Down
1 change: 1 addition & 0 deletions src/sphinx_alter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ void AddToSchema ( CSphSchema & tSchema, const AttrAddRemoveCtx_t & tCtx, CSphSt
tInfo.m_uAttrFlags = tCtx.m_uFlags;
tInfo.m_eEngine = tCtx.m_eEngine;
tInfo.m_tLocator.m_iBitCount = tCtx.m_iBits;
tInfo.m_tKNN = tCtx.m_tKNN;

auto iIdxExisting = tSchema.GetAttrIndex ( tCtx.m_sName.cstr() );
if ( iIdxExisting >= 0 )
Expand Down
1 change: 1 addition & 0 deletions src/sphinx_alter.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ struct AttrAddRemoveCtx_t
int m_iBits;
DWORD m_uFlags;
AttrEngine_e m_eEngine;
knn::IndexSettings_t m_tKNN;
};

// common add/remove attribute/field code for both RT and plain indexes
Expand Down
9 changes: 7 additions & 2 deletions test/test_275/model.bin

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions test/test_275/test.xml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ searchd
select id, knn_dist() from test where knn ( image_vector, 5, (0.286569,-0.031816,0.066684,0.032926) );
flush ramchunk test;
select id, knn_dist() from test where knn ( image_vector, 5, (0.286569,-0.031816,0.066684,0.032926) );

<!-- regression knn option missed at alter table -->
alter table test add column image_vecto2 float_vector knn_type='hnsw' knn_dims='7' hnsw_similarity='l2';
show create table test;

drop table test;
</sphinxql>
</queries>
Expand Down

0 comments on commit 3d5014e

Please sign in to comment.