Poco::Data::MySql::ResultMetadata::adjustColumnSizeToFit(std::size_t pos)

[micheleselea]

I have a problem with Poco::Data::MySql::ResultMetadata::adjustColumnSizeToFit(std::size_t pos) since versione 1.12.1.
The original implementation it's

void ResultMetadata::adjustColumnSizeToFit(std::size_t pos)
{
	std::free(_buffer[pos]);
	_buffer[pos] = (char*) std::calloc(_lengths[pos], sizeof(char));
	_row[pos].buffer = _buffer[pos];
	_row[pos].buffer_length = _lengths[pos];
}

This implementation give me a problem of accesso violation in function

bool StatementExecutor::fetch()
{
	if (_state < STMT_EXECUTED)
		throw StatementException("Statement is not executed yet");

	int res = mysql_stmt_fetch(_pHandle); <---ACCESS VIOLATION

	// we have specified zero buffers for BLOBs, so DATA_TRUNCATED is normal in this case
	if ((res != 0) && (res != MYSQL_NO_DATA) && (res != MYSQL_DATA_TRUNCATED))
		throw StatementException("mysql_stmt_fetch error", _pHandle, _query);

	return (res == 0) || (res == MYSQL_DATA_TRUNCATED);
}

Tha is a result of a simply Select like this:

std::string sQuery = "SELECT DateTime,SensorType,SensorValue,Description FROM system_stats WHERE SensorType IN (6) AND DateTime>1785497698191598 AND DateTime<1785501298191598 ORDER BY DateTime ASC";
std::vector<Poco::Tuple<Poco::Int64, Poco::Int64, Poco::Int64, std::string>> sysStatsResults;
sLogsDB << sQuery, Poco::Data::Keywords::into(sysStatsResults), Poco::Data::Keywords::now;

so I think quite standard stuff (fields are BIGINT BIGINT BIGINT TEXT
if I rewrite the function like this

void ResultMetadata::adjustColumnSizeToFit(std::size_t pos)
{
	if (_row[pos].buffer_length > 0) return;
	if (_buffer[pos] != nullptr) std::free(_buffer[pos]);
	_buffer[pos] = (char*) std::calloc(_lengths[pos], sizeof(char));
	if (_buffer[pos] == nullptr) _lengths[pos] = 0;
	_row[pos].buffer = _buffer[pos];
	_row[pos].buffer_length = _lengths[pos];
}

Select is working, I don't know why no one else has ever get this problem, but I can't figure out if can be my code problem

[micheleselea]

we already speak of this
#3841

[micheleselea]

Even I think this
if (_row[pos].buffer_length > 0) return;
it's conceptually wrong I think.
Anyway the reason is called adjustColumnSizeToFit it's because the column is detected as Large LOBs (LONGBLOB and LONGTEXT) that is not because is a TEXT but in mysql reports TEXT types as FDT_BLOB when being extracte

[micheleselea]

Ok a couple of things about. I did the function like this

void ResultMetadata::adjustColumnSizeToFit(std::size_t pos)
{
	std::size_t sizeNeeded = _lengths[pos];
	if (sizeNeeded > 100 * 1024 * 1024) {
		throw MySQLException("adjustColumnSizeToFit invalid length");
	}

	if (_row[pos].buffer_length < sizeNeeded)
	{
		std::free(_buffer[pos]);
		_buffer[pos] = (char*)std::calloc(sizeNeeded, sizeof(char));

		if (_buffer[pos] == nullptr) {
			throw MySQLException("adjustColumnSizeToFit calloc error");
		}

		_row[pos].buffer = _buffer[pos];
		_row[pos].buffer_length = static_cast<unsigned long>(sizeNeeded);
	}
	else
	{
		std::memset(_buffer[pos], 0, sizeNeeded);
	}
}

This way if you already have enough space in buffer_length there is no need to realloc.
This comment

bool Extractor::extractLongLOB(std::size_t pos)
{
	// Large LOBs (LONGBLOB and LONGTEXT) are fetched
	// with a zero-length buffer to avoid allocating
	// huge amounts of memory. Therefore, when extracting
	// the buffers need to be adjusted.

	_metadata.adjustColumnSizeToFit(pos);

	MYSQL_BIND* row = _metadata.row();
	if (!_stmt.fetchColumn(pos, &row[pos]))
		return false;

	return true;
}

it's not true because I never end up with a buffer_length of 0 so maybe we can optimize this.

Anyway the real core problem is the crash, now it's not crashing but it's just because I never enter the if
if (_row[pos].buffer_length < sizeNeeded)
Because it's already allocated.
The problem doing realloc is that I think we should do another mysql_stmt_bind_result after changing the _row structure to prevent crash. I'm diggin in that. Anyway I realy do no understand how is it possible that only me has this problem: maybe you never use TEXT column in MySQL?

[micheleselea]

Ok fixed I did something like:

bool ResultMetadata::adjustColumnSizeToFit(std::size_t pos)
{
	std::size_t sizeNeeded = _lengths[pos];
	if (sizeNeeded > 100 * 1024 * 1024) {
		throw MySQLException("adjustColumnSizeToFit invalid length");
	}

	if (_row[pos].buffer_length < sizeNeeded)
	{
		if (_buffer[pos] != nullptr) std::free(_buffer[pos]);
		_buffer[pos] = (char*)std::calloc(sizeNeeded, sizeof(char));

		if (_buffer[pos] == nullptr) {
			throw MySQLException("adjustColumnSizeToFit calloc error");
		}

		_row[pos].buffer = _buffer[pos];
		_row[pos].buffer_length = static_cast<unsigned long>(sizeNeeded);

		return true;
	}
	else
	{
		//std::memset(_buffer[pos], 0, sizeNeeded);
	}
	return false;
}

and when we call adjust in

bool Extractor::extractLongLOB(std::size_t pos)
{
	// Large LOBs (LONGBLOB and LONGTEXT) are fetched
	// with a zero-length buffer to avoid allocating
	// huge amounts of memory. Therefore, when extracting
	// the buffers need to be adjusted.

	if(_metadata.adjustColumnSizeToFit(pos))
		_stmt.bindResult(_metadata.row());

	MYSQL_BIND* row = _metadata.row();
	if (!_stmt.fetchColumn(pos, &row[pos]))
		return false;

	return true;
}

#ifdef POCO_MYSQL_JSON
bool Extractor::extractJSON(std::size_t pos)
{
	// JSON columns are fetched with a zero-length
	// buffer to avoid allocating huge amounts of memory.
	// Therefore, when extracting the buffers need to be adjusted.

	if(_metadata.adjustColumnSizeToFit(pos))
		_stmt.bindResult(_metadata.row());

	MYSQL_BIND* row = _metadata.row();
	row->buffer_type = MYSQL_TYPE_JSON;
	if (!_stmt.fetchColumn(pos, &row[pos]))
		return false;

	return true;
}
#endif

there is another optimization we can do, probably for LOBs we really do not need to allocate buffer too early because we can end up allocating 50MB just for 100bytes

[micheleselea]

ok the optimization for allocation can be done like that (look at switch (fields[i].type){ )

void ResultMetadata::init(MYSQL_STMT* stmt)
{
	ResultMetadataHandle h(stmt);

	if (!h)
	{
		// some queries (eg. INSERT) don't have result
		reset();
		return;
	}

	std::size_t count = mysql_num_fields(h);
	MYSQL_FIELD* fields = mysql_fetch_fields(h);

	_columns.reserve(count);

	for (std::size_t i = 0; i < count; i++)
	{
		std::size_t size = fieldSize(fields[i]);
		if (size == 0xFFFFFFFF) size = 0;		

		_columns.push_back(MetaColumn(
			i,                               // position
			fields[i].name,                  // name
			fieldType(fields[i]),            // type
			size,                            // length
			0,                               // TODO: precision
			!IS_NOT_NULL(fields[i].flags)    // nullable
			));
	}

	_buffer.resize(count);
	_row.resize(count);
	_lengths.resize(count);
	_isNull.resize(count);

	for (std::size_t i = 0; i < count; i++)
	{
		std::memset(&_row[i], 0, sizeof(MYSQL_BIND));
		unsigned int len;
		switch (fields[i].type) {
		default:len = static_cast<unsigned int>(_columns[i].length()); break;
		case MYSQL_TYPE_TINY_BLOB:
		case MYSQL_TYPE_MEDIUM_BLOB:
		case MYSQL_TYPE_LONG_BLOB:
		case MYSQL_TYPE_BLOB:
			len = 0; break;
#ifdef POCO_MYSQL_JSON
		case MYSQL_TYPE_JSON:
			len = 0; break;
#endif
		}
		if (len > 0) {
			_buffer[i] = (char*)std::calloc(len, sizeof(char));
			if (_buffer[i] == nullptr) len = 0;
		}
		else {
			_buffer[i] = nullptr;
		}
		_row[i].buffer_type   = fields[i].type;
		_row[i].buffer_length = len;
		_row[i].buffer        = _buffer[i];
		_row[i].length        = &_lengths[i];
		_row[i].is_null       = reinterpret_cast<my_bool*>(&_isNull[i]); // workaround to make it work with both MySQL 8 and earlier
		_row[i].is_unsigned   = (fields[i].flags & UNSIGNED_FLAG) > 0;
	}
}

[micheleselea]

The only way I get it working now is this, so to allocate the memory even for the blob in advance, that is what the library do right now, but is not optimized because we end up allocating 50MB for a 100byte text, I don't like it

void ResultMetadata::init(MYSQL_STMT* stmt)
{
	ResultMetadataHandle h(stmt);

	if (!h)
	{
		// some queries (eg. INSERT) don't have result
		reset();
		return;
	}

	std::size_t count = mysql_num_fields(h);
	MYSQL_FIELD* fields = mysql_fetch_fields(h);

	_columns.reserve(count);

	for (std::size_t i = 0; i < count; i++)
	{
		std::size_t size = fieldSize(fields[i]);
		if (size == 0xFFFFFFFF) size = 0;		

		_columns.push_back(MetaColumn(
			i,                               // position
			fields[i].name,                  // name
			fieldType(fields[i]),            // type
			size,                            // length
			0,                               // TODO: precision
			!IS_NOT_NULL(fields[i].flags)    // nullable
			));
	}

	_buffer.resize(count);
	_row.resize(count);
	_lengths.resize(count);
	_isNull.resize(count);

	for (std::size_t i = 0; i < count; i++)
	{
		std::memset(&_row[i], 0, sizeof(MYSQL_BIND));
		unsigned int len = static_cast<unsigned int>(_columns[i].length());
		if (len > 0) {
			_buffer[i] = (char*)std::calloc(len, sizeof(char));
			if (_buffer[i] == nullptr) len = 0;
		}
		else {
			_buffer[i] = nullptr;
		}
		_row[i].buffer_type   = fields[i].type;
		_row[i].buffer_length = len;
		_row[i].buffer        = _buffer[i];
		_row[i].length        = &_lengths[i];
		_row[i].is_null       = reinterpret_cast<my_bool*>(&_isNull[i]); // workaround to make it work with both MySQL 8 and earlier
		_row[i].is_unsigned   = (fields[i].flags & UNSIGNED_FLAG) > 0;
	}
}

[micheleselea]

Anyway the main concern is:

1. Because the adjustColumnSizeToFit function is always called when extractLongLOB ad extractJSON
2. And the original implementation just reallocate _buffer[pos] no matter is the size, without rebinding
3. How did you never get error on fetch?

we have to deal better with blob and DATA_TRUNCATED