I'll look to see if this still fails after Mytherins PR is merged

Hey! Thanks for the above work everyone. Is this one gunna be fixed in the next release then?

Hey! Thanks for the above work everyone. Is this one gunna be fixed in the next release then?

The fix itself is already included in the latest dev builds

I have just tried running the test on:
#5670

With the latest dev build, which I assume is what is installed via:
npm install duckdb@next

And it still fails in the same way. Are we sure this issue has been addressed?

Could we get this PR merged to confirm it really is fixed on master branch @Mytherin / @Mause?

@Mytherin - Thanks, if I am reading it correctly though the action failed on this test:

Yea it seems this wasn't fixed by the other PR and the issue still exists

So the issue seems to be that we use the c_str() of a temporary string, which goes out of scope and the memory becomes random.

		case duckdb::LogicalTypeId::BLOB:
		case duckdb::LogicalTypeId::VARCHAR: {
			auto data = ret.Get("data").As<Napi::Array>();
			auto out = duckdb::FlatVector::GetData<duckdb::string_t>(*jsargs->result);
			for (size_t i = 0; i < data.Length(); ++i) {
				out[i] = duckdb::string_t(data.Get(i).ToString());
			}
			break;
		}

This ends up in this constructor

	string_t(const string &value)
	    : string_t(value.c_str(), value.size()) { // NOLINT: Allow implicit conversion from `const char*`
	}

And for non-inlined strings we just copy the pointer

			// large string: store pointer
#ifndef DUCKDB_DEBUG_NO_INLINE
			memcpy(value.pointer.prefix, data, PREFIX_LENGTH);
#else
			memset(value.pointer.prefix, 0, PREFIX_BYTES);
#endif
			value.pointer.ptr = (char *)data;
		}

This change should fix the issue 👍

StringVector's have a StringHeap exactly for this purpose, we need to make sure the memory of these non-inlined strings stays alive long enough, so we copy over the memory into this heap and the data of the vector contains a string_t that points into this heap.

Thanks! Looks good to me.