Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rfc89 sql logging callback implementation #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Rfc89 sql logging callback implementation #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
Binary file added autotest/cpp/data/poly-1-feature.gpkg
View file
Open in desktop
Binary file not shown.
91 changes: 91 additions & 0 deletions autotest/cpp/test_ogr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
#include "ogr_recordbatch.h"

#include <string>
#include <algorithm>
#include <fstream>

#include "gtest_include.h"

Expand Down Expand Up @@ -2163,4 +2165,93 @@ namespace
EXPECT_EQ(i, oFDefn.GetGeomFieldCount());
}

// Test GDALDataset QueryLoggerFunc callback
TEST_F(test_ogr, GDALDatasetSetQueryLoggerFunc)
{
if( GDALGetDriverByName("GPKG") == nullptr )
{
GTEST_SKIP() << "GPKG driver missing";
}

auto tmpGPKG { testing::TempDir() + "/poly-1-feature.gpkg" };
{
std::ifstream src("data/poly-1-feature.gpkg", std::ios::binary);
std::ofstream dst(tmpGPKG, std::ios::binary);
dst << src.rdbuf();
}

auto poDS = std::unique_ptr<GDALDataset>(GDALDataset::Open(tmpGPKG.c_str(), GDAL_OF_VECTOR|GDAL_OF_UPDATE));
ASSERT_TRUE( poDS );
auto hDS = GDALDataset::ToHandle( poDS.get() );
ASSERT_TRUE( hDS );

struct QueryLogEntry
{
std::string sql;
std::string error;
int64_t numRecords;
int64_t executionTimeMilliseconds;
};

std::vector<QueryLogEntry> queryLog;
const bool retVal = GDALDatasetSetQueryLoggerFunc( hDS , [ ] (const char *pszSQL, const char *pszError, int64_t lNumRecords, int64_t lExecutionTimeMilliseconds, void *pQueryLoggerArg)
{
std::vector<QueryLogEntry>* queryLogLocal { reinterpret_cast<std::vector<QueryLogEntry>*>( pQueryLoggerArg ) };
QueryLogEntry entryLocal;
entryLocal.sql = pszSQL;
entryLocal.numRecords = lNumRecords;
entryLocal.executionTimeMilliseconds = lExecutionTimeMilliseconds;
if ( pszError )
{
entryLocal.error = pszError;
}
queryLogLocal->push_back( entryLocal );
}, &queryLog );

ASSERT_TRUE( retVal );
auto hLayer { GDALDatasetGetLayer( hDS, 0 ) };
ASSERT_TRUE( hLayer );
ASSERT_STREQ( OGR_L_GetName( hLayer ), "poly" );
auto poFeature = std::unique_ptr<OGRFeature>(OGRFeature::FromHandle(OGR_L_GetNextFeature( hLayer )));
auto hFeature = OGRFeature::ToHandle(poFeature.get());
ASSERT_TRUE( hFeature );
ASSERT_GT( queryLog.size(), 1 );

QueryLogEntry entry { queryLog.back() };
ASSERT_EQ( entry.sql.find( "SELECT", 0 ), 0 );
ASSERT_TRUE( entry.executionTimeMilliseconds >= 0 );
ASSERT_EQ( entry.numRecords, -1 );
ASSERT_TRUE( entry.error.empty() );

// Test erroneous query
OGRLayerH queryResultLayerH { GDALDatasetExecuteSQL( hDS, "SELECT * FROM not_existing_table", nullptr, nullptr ) };
GDALDatasetReleaseResultSet( hDS, queryResultLayerH );
ASSERT_FALSE( queryResultLayerH );

entry = queryLog.back();
ASSERT_EQ( entry.sql.find( "SELECT * FROM not_existing_table", 0 ), 0 );
ASSERT_EQ( entry.executionTimeMilliseconds, -1 );
ASSERT_EQ( entry.numRecords, -1 );
ASSERT_FALSE( entry.error.empty() );

// Test prepared arg substitution
hFeature = OGR_F_Create( OGR_L_GetLayerDefn( hLayer ) );
poFeature.reset( OGRFeature::FromHandle( hFeature ) );
const char* wkt = "POLYGON ((479819 4765180,479690 4765259,479647 4765369, 479819 4765180))";
OGRGeometryH testGeom = nullptr;
OGRErr err = OGR_G_CreateFromWkt((char**) &wkt, nullptr, &testGeom);
ASSERT_EQ(OGRERR_NONE, err);
err = OGR_F_SetGeometryDirectly( hFeature, testGeom );
ASSERT_EQ(OGRERR_NONE, err);
err = OGR_L_CreateFeature( hLayer, hFeature );
ASSERT_EQ(OGRERR_NONE, err);

auto insertEntry = std::find_if( queryLog.cbegin(), queryLog.cend(), [](const QueryLogEntry& e) {
return e.sql.find( R"sql(INSERT INTO "poly")sql", 0 ) == 0;
});

ASSERT_TRUE( insertEntry != queryLog.end( ));
ASSERT_EQ( insertEntry->sql.find( R"sql(INSERT INTO "poly" ( "geom", "AREA", "EAS_ID", "PRFEDEA") VALUES (x'47500003346c0000000000007c461d41)sql", 0 ), 0 );
}

} // namespace
4 changes: 4 additions & 0 deletions gcore/gdal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,10 @@ bool CPL_DLL GDALDatasetUpdateRelationship(GDALDatasetH hDS,
GDALRelationshipH hRelationship,
char** ppszFailureReason);

typedef void (*GDALQueryLoggerFunc)(const char *pszSQL, const char *pszError, int64_t lNumRecords, int64_t lExecutionTimeMilliseconds, void *pQueryLoggerArg);

bool CPL_DLL GDALDatasetSetQueryLoggerFunc(GDALDatasetH hDS, GDALQueryLoggerFunc pfnQueryLoggerFunc, void* poQueryLoggerArg );

/* ==================================================================== */
/* GDALRasterBand ... one band/channel in a dataset. */
/* ==================================================================== */
Expand Down
3 changes: 3 additions & 0 deletions gcore/gdal_priv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ class CPL_DLL GDALDataset : public GDALMajorObject
void ReleaseMutex();
//! @endcond


public:
~GDALDataset() override;

Expand All @@ -477,6 +478,8 @@ class CPL_DLL GDALDataset : public GDALMajorObject
int GetRasterCount();
GDALRasterBand *GetRasterBand( int );

virtual bool SetQueryLoggerFunc(GDALQueryLoggerFunc pfnQueryLoggerFuncIn, void* poQueryLoggerArgIn );

/** Class returned by GetBands() that act as a container for raster bands. */
class CPL_DLL Bands
{
Expand Down
37 changes: 37 additions & 0 deletions gcore/gdaldataset.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7514,6 +7514,16 @@ void GDALDataset::ShareLockWithParentDataset(GDALDataset* poParentDataset)
}
}

/************************************************************************/
/* SetQueryLoggerFunc() */
/************************************************************************/


bool GDALDataset::SetQueryLoggerFunc(CPL_UNUSED GDALQueryLoggerFunc callback, CPL_UNUSED void* context )
Copy link

Choose a reason for hiding this comment

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

Doxygen comments to add

{
return false;
}

/************************************************************************/
/* EnterReadWrite() */
/************************************************************************/
Expand Down Expand Up @@ -8989,6 +8999,33 @@ bool GDALDatasetUpdateRelationship(GDALDatasetH hDS,
}


/************************************************************************/
/* GDALDatasetSetQueryLoggerFunc() */
/************************************************************************/

/**
* Sets the SQL query logger callback.
*
* When supported by the driver, the callback will be called with
* the executed SQL text, the error message, the execution time in milliseconds,
* the number of records fetched/affected and the client status data.
*
* A value of -1 in the execution time or in the number of records indicates
* that the values are unknown.
*
* @param hDS Dataset handle.
* @param pfnQueryLoggerFunc Callback function
* @param poQueryLoggerArg Opaque client status data
* @return true in case of success.
* @since GDAL 3.7
*/
bool GDALDatasetSetQueryLoggerFunc(GDALDatasetH hDS, GDALQueryLoggerFunc pfnQueryLoggerFunc , void* poQueryLoggerArg)
{
VALIDATE_POINTER1(hDS, __func__, false);
return GDALDataset::FromHandle(hDS)->SetQueryLoggerFunc( pfnQueryLoggerFunc, poQueryLoggerArg );
}


//! @cond Doxygen_Suppress

/************************************************************************/
Expand Down
4 changes: 2 additions & 2 deletions ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6607,12 +6607,12 @@ OGRLayer * GDALGeoPackageDataset::ExecuteSQL( const char *pszSQLCommand,
}
}

int rc = sqlite3_prepare_v2( hDB, osSQLCommandTruncated.c_str(),
int rc = prepareSql( hDB, osSQLCommandTruncated.c_str(),
static_cast<int>(osSQLCommandTruncated.size()),
&hSQLStmt, nullptr );

if( rc != SQLITE_OK )
{
{
Copy link

Choose a reason for hiding this comment

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

extra whitespace

CPLError( CE_Failure, CPLE_AppDefined,
"In ExecuteSQL(): sqlite3_prepare_v2(%s):\n %s",
osSQLCommandTruncated.c_str(), sqlite3_errmsg(hDB) );
Expand Down
7 changes: 7 additions & 0 deletions ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7125,6 +7125,13 @@ int OGRGeoPackageTableLayer::GetNextArrowArray(struct ArrowArrayStream* stream,
{
break;
}

// Install query logging callback
if ( m_poDS->pfnQueryLoggerFunc )
{
task->m_poDS->SetQueryLoggerFunc( m_poDS->pfnQueryLoggerFunc, m_poDS->poQueryLoggerArg );
}

task->m_poLayer = poOtherLayer;
task->m_psArrowArray = cpl::make_unique<struct ArrowArray>();
memset(task->m_psArrowArray.get(), 0, sizeof(struct ArrowArray));
Expand Down
15 changes: 15 additions & 0 deletions ogr/ogrsf_frmts/sqlite/ogrsqlitebase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ class OGRSQLiteBaseDataSource CPL_NON_FINAL: public GDALPamDataset

virtual std::pair<OGRLayer*, IOGRSQLiteGetSpatialWhere*> GetLayerWithGetSpatialWhereByName( const char* pszName ) = 0;

bool SetQueryLoggerFunc(GDALQueryLoggerFunc pfnQueryLoggerFuncIn, void*poQueryLoggerArgIn) override;

virtual OGRErr AbortSQL() override;

virtual OGRErr StartTransaction(int bForce = FALSE) override;
Expand All @@ -193,6 +195,19 @@ class OGRSQLiteBaseDataSource CPL_NON_FINAL: public GDALPamDataset
OGRErr PragmaCheck(const char * pszPragma, const char * pszExpected, int nRowsExpected);

void LoadRelationshipsFromForeignKeys() const;

// sqlite3_prepare_v2 error logging wrapper
int prepareSql(
sqlite3 *db, /* Database handle */
const char *zSql, /* SQL statement, UTF-8 encoded */
int nByte, /* Maximum length of zSql in bytes. */
sqlite3_stmt **ppStmt, /* OUT: Statement handle */
const char **pzTail /* OUT: Pointer to unused portion of zSql */
);

GDALQueryLoggerFunc pfnQueryLoggerFunc = nullptr;
void* poQueryLoggerArg = nullptr;

};

/************************************************************************/
Expand Down
61 changes: 56 additions & 5 deletions ogr/ogrsf_frmts/sqlite/ogrsqlitedatasource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,22 @@ void OGRSQLiteBaseDataSource::LoadRelationshipsFromForeignKeys() const
#endif
}

/***********************************************************************/
/* prepareSql() */
/***********************************************************************/

int OGRSQLiteBaseDataSource::prepareSql(sqlite3*db, const char*zSql, int nByte, sqlite3_stmt**ppStmt, const char**pzTail)
{
const int rc { sqlite3_prepare_v2( db, zSql, nByte, ppStmt, pzTail ) };
if ( rc != SQLITE_OK && pfnQueryLoggerFunc )
{
std::string error { "Error preparing query: " };
error.append( sqlite3_errmsg( db ) );
pfnQueryLoggerFunc( zSql, error.c_str(), -1, -1, poQueryLoggerArg );
}
return rc;
}


/************************************************************************/
/* OGRSQLiteDataSource() */
Expand Down Expand Up @@ -1640,7 +1656,7 @@ bool OGRSQLiteDataSource::InitWithEPSG()
}

sqlite3_stmt *hInsertStmt = nullptr;
rc = sqlite3_prepare_v2( hDB, osCommand, -1, &hInsertStmt, nullptr );
rc = prepareSql( hDB, osCommand, -1, &hInsertStmt, nullptr );

if ( pszProjCS )
{
Expand Down Expand Up @@ -1705,7 +1721,7 @@ bool OGRSQLiteDataSource::InitWithEPSG()
nSRSId, nSRSId );

sqlite3_stmt *hInsertStmt = nullptr;
rc = sqlite3_prepare_v2( hDB, osCommand, -1, &hInsertStmt, nullptr );
rc = prepareSql( hDB, osCommand, -1, &hInsertStmt, nullptr );

if( rc == SQLITE_OK)
rc = sqlite3_bind_text( hInsertStmt, 1, pszWKT, -1, SQLITE_STATIC );
Expand Down Expand Up @@ -2905,7 +2921,7 @@ OGRLayer * OGRSQLiteDataSource::ExecuteSQL( const char *pszSQLCommand,
}
}

int rc = sqlite3_prepare_v2( GetDB(), osSQLCommand.c_str(),
int rc = prepareSql( GetDB(), osSQLCommand.c_str(),
static_cast<int>(osSQLCommand.size()),
&hSQLStmt, nullptr );

Expand Down Expand Up @@ -3874,7 +3890,7 @@ int OGRSQLiteDataSource::FetchSRSId( const OGRSpatialReference * poSRS )
}

sqlite3_stmt *hSelectStmt = nullptr;
int rc = sqlite3_prepare_v2( hDB, osCommand, -1, &hSelectStmt, nullptr );
int rc = prepareSql( hDB, osCommand, -1, &hSelectStmt, nullptr );

if( rc == SQLITE_OK)
rc = sqlite3_bind_text( hSelectStmt, 1,
Expand Down Expand Up @@ -4088,7 +4104,7 @@ int OGRSQLiteDataSource::FetchSRSId( const OGRSpatialReference * poSRS )
}

sqlite3_stmt *hInsertStmt = nullptr;
rc = sqlite3_prepare_v2( hDB, osCommand, -1, &hInsertStmt, nullptr );
rc = prepareSql( hDB, osCommand, -1, &hInsertStmt, nullptr );

for( int i = 0; apszToInsert[i] != nullptr; i++ )
{
Expand Down Expand Up @@ -4317,6 +4333,41 @@ void OGRSQLiteBaseDataSource::SetEnvelopeForSQL(const CPLString& osSQL,
oMapSQLEnvelope[osSQL] = oEnvelope;
}

/***********************************************************************/
/* SetQueryLoggerFunc() */
/***********************************************************************/

bool OGRSQLiteBaseDataSource::SetQueryLoggerFunc(GDALQueryLoggerFunc pfnQueryLoggerFuncIn, void*poQueryLoggerArgIn)
{

#if SQLITE_VERSION_NUMBER < 3014000
return false;
#endif

pfnQueryLoggerFunc = pfnQueryLoggerFuncIn;
poQueryLoggerArg = poQueryLoggerArgIn;

if ( pfnQueryLoggerFunc )
{
sqlite3_trace_v2( hDB, SQLITE_TRACE_PROFILE, [ ](unsigned int /* traceProfile */, void* context, void* preparedStatement, void* executionTime ) -> int
{
if ( context )
{
const std::string sql { sqlite3_expanded_sql( reinterpret_cast<sqlite3_stmt *>( preparedStatement ) ) };
const uint64_t executionTimeMilliSeconds { static_cast<uint64_t>( *reinterpret_cast<uint64_t*>( executionTime ) / 1e+6 ) };
OGRSQLiteBaseDataSource* source { reinterpret_cast<OGRSQLiteBaseDataSource*>( context ) };
if ( source->pfnQueryLoggerFunc )
{
source->pfnQueryLoggerFunc( sql.c_str(), nullptr, -1, executionTimeMilliSeconds, source->poQueryLoggerArg );
}
}
return 0;
}, reinterpret_cast<void *>( this ) );
return true;
}
return false;
}

/************************************************************************/
/* AbortSQL() */
/************************************************************************/
Expand Down